Closed Bug 1248534 Opened 4 years ago Closed 4 years ago

Remove unused XPT features

Categories

(Core :: XPCOM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: njn, Assigned: njn)

References

Details

Attachments

(9 files, 1 obsolete file)

75.25 KB, patch
khuey
: review+
Details | Diff | Splinter Review
4.32 KB, patch
khuey
: review+
Details | Diff | Splinter Review
10.25 KB, patch
khuey
: review+
Details | Diff | Splinter Review
4.47 KB, patch
khuey
: review+
Details | Diff | Splinter Review
4.39 KB, patch
khuey
: review+
Details | Diff | Splinter Review
1.90 KB, patch
khuey
: review+
Details | Diff | Splinter Review
17.58 KB, patch
khuey
: review+
Details | Diff | Splinter Review
11.00 KB, patch
khuey
: review+
Details | Diff | Splinter Review
1.83 KB, patch
khuey
: review+
Details | Diff | Splinter Review
We can remove some XPT code.
Currently XPT can both encode and decode, but encoding has been handled by
Python code since bug 643817, so the encoding support can be removed. This
results in many simplifications. Some notable changes:

- All the XPTHashTable code (including XPTDatapool::offset_map) is no longer
  necessary.

- PrimitiveTest.cpp and SimpleTypeLib.cpp both don't make much sense without
  encoding support, so I removed them.

- A lot of the version code was already unused, e.g. XPT_VERSION_*,
  XPT_TYPELIB_VERSIONS_STRUCT, XPT_TYPELIB_VERSIONS.
  XPT_MAJOR_INCOMPATIBLE_VERSION is the only thing actually used in version
  checks.

- The patch also removes some code that was dead even before encoding removal,
  such as XPT_ParseVersionString().
Attachment #8719682 - Flags: review?(khuey)
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
For part 1 we have:

 9 files changed, 107 insertions(+), 1423 deletions(-)
FWIW, while I will do these reviews, they are quite low priority to me.
The XPT_ENCODE removal was inspired by part 1 of bug 1249174 -- removing the argnum2 field is easier when we don't have to worry about encoding.
PT supports annotations but xpt.py doesn't generate them except for a single
empty annotation (to indicate there are no real annotations). So we can remove
almost all support for them. This also allows XPTString to be removed.
Attachment #8720646 - Flags: review?(khuey)
It can just be inlined into XPTState, which simplifies things.
Attachment #8721161 - Flags: review?(khuey)
RegisterBuffer() is the only place that creates an XPTState, and it also
destroys it. So the XPTState can be allocated on the stack, which voids the
need for the creation of an XPTArena.
Attachment #8721162 - Flags: review?(khuey)
PT has some functions and macros for freeing memory. However, they (a) are
only used on error paths, and (b) don't actually free memory -- they just
optionally log the "freeing" -- because piecewise freeing doesn't make sense
with arena allocation.

This patch removes all that unnecessary machinery.
Attachment #8721814 - Flags: review?(khuey)
I bet nobody has used this code in a decade. I haven't, even though I've been
working on reducing XPT memory usage. Well-targeted diagnostic printfs are much
more useful than these kind of overview stats, in my experience.
Attachment #8721839 - Flags: review?(khuey)
These 9 patches reduce the number of lines of code in xpcom/typelib/xpt/*.{h,cpp} from 2680 to 1316, and the diffstat overall is as follows:

 13 files changed, 142 insertions(+), 1805 deletions(-)
Summary: Removed unused XPT features → Remove unused XPT features
Attachment #8720647 - Attachment is obsolete: true
Attachment #8720647 - Flags: review?(khuey)
Whoops, part 7 didn't get mentioned in comment 15 because I mistakenly put bug 1249174 in the commit message. No matter. See also bug 1249174 comment 15.
Depends on: 1251298
You need to log in before you can comment on or make changes to this bug.