Closed Bug 1443230 Opened 2 years ago Closed 2 years ago

Keep hashtable of interface indexes in xpt.py

Categories

(Core :: XPCOM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: mccr8, Assigned: mccr8)

References

Details

Attachments

(1 file)

I looked at a profile of running xpt.py on a final link that includes all XPT files. This was for a variant that outputs C++ instead of an XPT file, but I think the performance should be similar. The base time was 2.7 seconds.

I noticed a lot of the running time was spent calling a list index, which is done to find the index of an interface. Replacing these index calls with a map from interfaces to their indexes reduced the running time to 1 second.

There are also some calls to |interface in self.interfaces| in _sanityCheck which can also be replaced by using this same index map. That reduced the running time further to 0.9 seconds.

This does make the code more fragile, because the map has to be updated in parallel with the list, but it doesn't seem too bad to me.
Comment on attachment 8956213 [details]
Bug 1443230 - Keep hashtable of interface indexes in xpt.py.

https://reviewboard.mozilla.org/r/225118/#review231208

I think I'd prefer a wrapping type for interfaces, that pretends to be the list type it replaces, rather than relying on callers doing the right thing.

Something like:

class IndexedList(object):
    def __init__(self, iterable):
        self._list = []
        self._index_map = {}
        for i in iterable:
            self.append(i)

    def sort(self):
        self._list.sort()
        self._index_map = {val: i for i, val in enumerate(self._list, 1)}

    def append(self, val):
        self._index_map[val] = len(self._list)]
        self._list.append(val)

    def index(self, what):
        return self._index_map[what]

    def __contains__(self, what):
        return what in self._index_map

    def __iter__(self):
        return iter(self._list)

    def __getitem__(self, index):
        return self._list[index]

    def __len__(self):
        return len(self._list)
Attachment #8956213 - Flags: review?(mh+mozilla)
(In reply to Mike Hommey [:glandium] from comment #2)
> I think I'd prefer a wrapping type for interfaces, that pretends to be the
> list type it replaces, rather than relying on callers doing the right thing.

Oh, wow, that's much better.
I confirmed locally that I saw the same performance improvements in a regular build without my XPT patches. 

try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6a116128b75b03bc81aa011f8cf0ae8e34985da2
Comment on attachment 8956213 [details]
Bug 1443230 - Keep hashtable of interface indexes in xpt.py.

https://reviewboard.mozilla.org/r/225118/#review231440

::: xpcom/typelib/xpt/tools/xpt.py:112
(Diff revision 2)
> +    def __init__(self, iterable):
> +        self._list = []
> +        self._index_map = {}
> +        for i in iterable:
> +            self.append(i)
> +        self._generate_index_map()

You don't need this, since you're going through append() to add the contents of iterable, so the index map will already be filled for each elements in there.
Attachment #8956213 - Flags: review?(mh+mozilla) → review+
(In reply to Mike Hommey [:glandium] from comment #6)
> You don't need this, since you're going through append() to add the contents
> of iterable, so the index map will already be filled for each elements in
> there.

Oops, right. I'm not sure why I didn't see that.
Pushed by amccreight@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/08802a2ae2b5
Keep hashtable of interface indexes in xpt.py. r=glandium
Blocks: 1438688
https://hg.mozilla.org/mozilla-central/rev/08802a2ae2b5
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.