Migrate Services.jsm to C++ and include common constructors
Categories
(Core :: XPCOM, enhancement)
Tracking
()
Tracking | Status | |
---|---|---|
firefox80 | --- | fixed |
People
(Reporter: kmag, Assigned: kmag)
References
(Blocks 1 open bug)
Details
(Keywords: perf-alert, Whiteboard: [overhead:20k])
Attachments
(6 files)
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review |
According to the results in bug 1463569, we use a fair amount of memory in every process allocating reflectors for objects in Cc and Ci. If we migrate Services.jsm to C++, we won't need to touch the JS reflectors for those classes or interfaces. And if we also add constructors for common classes that we createInstance, that should reduce the number of reflectors we need to create even more (to say nothing of cleaning up all of the XPCOM gunk that's currently required to create those instances).
Comment 1•6 years ago
|
||
Kris, do you have any idea how much overhead this is?
Assignee | ||
Comment 2•6 years ago
|
||
It's hard to say. Just going by the retained size of Services.jsm and the various Cc and Ci classes from mccr8's memory tool, I'd say somewhere between 50 and 100K in a base content process. Fully realizing that probably means making sure we don't touch Ci more than we have to, and probably optimizing generateQI to lookup IIDs directly rather than going through Ci.
Updated•6 years ago
|
Assignee | ||
Comment 3•6 years ago
|
||
Looking again at the most recent output, this is probably closer to 20-50K. But I'm pretty confident of at least 20 if we can actually get rid of most of the Ci/Cc references and get everything using Services.
Comment 4•6 years ago
|
||
Ideally this could be merged into the c++ generated from Services.py so C++ and JS code can share caches. https://searchfox.org/mozilla-central/source/xpcom/build/Services.py FYI I also have some WIP patches which should make Ci and Cc much cheaper in terms of both performance and memory use, so some of that overhead might disappear.
Comment 5•6 years ago
|
||
Note: if we do this, ideally this should be done in a way that makes it easy for ESLint to know what services are exposed as we already have a rule to ensure the use of it where possible: https://searchfox.org/mozilla-central/rev/3fdc491e118c5cdfbaf6e2d52f3466d2b27ad1de/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/use-services.js
Updated•6 years ago
|
Comment 6•6 years ago
|
||
FWIW Anny's partial work in progress patch for this bug is posted here: https://bug1489024.bmoattachments.org/attachment.cgi?id=9015611
Updated•4 years ago
|
Assignee | ||
Comment 7•4 years ago
|
||
This makes it much simpler for the for the static JS services cache to store
and lookup interface IDs for components.
Assignee | ||
Comment 8•4 years ago
|
||
This builds on the existing static components infrastructure to allow defining
a Services.jsm-type services cache with no runtime memory overhead for any
services until they're accessed.
Any class entry with a 'js_name' attribute automatically becomes available on
the services cache with that name, and any interfaces listed in its
'interfaces' list are automatically queried on it.
Assignee | ||
Comment 9•4 years ago
|
||
Aside from making registration somewhat more efficient, this allows us to make
the services available on the new C++-implemented JS Services object, which
requires services to use the new static component registration system.
Assignee | ||
Comment 10•4 years ago
|
||
Assignee | ||
Comment 11•4 years ago
|
||
Comment 12•4 years ago
|
||
Pushed by maglione.k@gmail.com: https://hg.mozilla.org/integration/autoland/rev/bbecc16d08eb Part 1 - Generate symbolic names for XPT interfaces and add lookup function. r=nika,froydnj https://hg.mozilla.org/integration/autoland/rev/e84de1547c09 Part 2 - Add infrastructure for defining named services exposed to JS. r=mccr8 https://hg.mozilla.org/integration/autoland/rev/51ff93220a95 Part 3a - Modernize enterprise policy registration. r=mkaply https://hg.mozilla.org/integration/autoland/rev/71c3475fcbc2 Part 3b - Add existing Services.jsm registrations to the new services cache. r=mccr8,geckoview-reviewers,agi https://hg.mozilla.org/integration/autoland/rev/9d3a0ea2cf65 Part 3c - Change Services.jsm to use the C++-implemented services cache. r=mccr8
Assignee | ||
Comment 13•4 years ago
|
||
Comment 14•4 years ago
|
||
Pushed by maglione.k@gmail.com: https://hg.mozilla.org/integration/autoland/rev/b50af9005851 Temporarily remove user-services.js test to fix bustage.
Comment 15•4 years ago
|
||
Backed out for xpcshell failures on test_Services.js
backout: https://hg.mozilla.org/integration/autoland/rev/07ab7d77bf31bbf3e74f8aab2b4cccf29b971681
failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=309191685&repo=autoland&lineNumber=5513
Comment 16•4 years ago
•
|
||
Comment 17•4 years ago
|
||
Pushed by maglione.k@gmail.com: https://hg.mozilla.org/integration/autoland/rev/e3dc107c8af8 Part 1 - Generate symbolic names for XPT interfaces and add lookup function. r=nika,froydnj https://hg.mozilla.org/integration/autoland/rev/98b3ba622432 Part 2 - Add infrastructure for defining named services exposed to JS. r=mccr8 https://hg.mozilla.org/integration/autoland/rev/f583d66b2813 Part 3a - Modernize enterprise policy registration. r=mkaply https://hg.mozilla.org/integration/autoland/rev/4fdeb4d64741 Part 3b - Add existing Services.jsm registrations to the new services cache. r=mccr8,geckoview-reviewers,agi https://hg.mozilla.org/integration/autoland/rev/671c2fd7e5a9 Part 3c - Change Services.jsm to use the C++-implemented services cache. r=mccr8
Comment 18•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e3dc107c8af8
https://hg.mozilla.org/mozilla-central/rev/98b3ba622432
https://hg.mozilla.org/mozilla-central/rev/f583d66b2813
https://hg.mozilla.org/mozilla-central/rev/4fdeb4d64741
https://hg.mozilla.org/mozilla-central/rev/671c2fd7e5a9
Comment 19•4 years ago
|
||
Pushed by richard.marti@gmail.com: https://hg.mozilla.org/comm-central/rev/753b43f99f9f Port bug 1464542 Part 3a - Modernize enterprise policy registration. rs=bustage-fix
Comment 20•4 years ago
|
||
== Change summary for alert #26476 (as of Fri, 10 Jul 2020 13:05:53 GMT) ==
Improvements:
1% Base Content JS macosx1014-64-shippable opt 3,551,187.00 -> 3,507,422.00
1% Base Content JS windows10-64-shippable opt 3,603,884.33 -> 3,567,582.00
1% Base Content JS windows10-64-shippable-qr opt 3,588,425.33 -> 3,554,250.00
1% Base Content JS windows7-32-shippable opt 2,761,914.67 -> 2,735,881.33
1% Base Content JS linux1804-64-shippable opt 3,527,639.67 -> 3,500,949.33
1% Base Content JS linux1804-64-shippable-qr opt 3,527,635.33 -> 3,500,932.00
For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=26476
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Comment 21•4 years ago
•
|
||
This is great!
I'm wondering if we can use something like what mccr8 did in bug 767640 to make the property always available, instead of everywhere having to manually ChromeUtils.import
Services.jsm ?
(Happy to file a follow-up bug, but I don't know if there was some reason it didn't happen here that I should be aware of.)
Assignee | ||
Comment 22•4 years ago
|
||
(In reply to :Gijs (he/him) from comment #21)
I'm wondering if we can use something like what mccr8 did in bug 767640 to make the property always available, instead of everywhere having to manually
ChromeUtils.import
Services.jsm ?(Happy to file a follow-up bug, but I don't know if there was some reason it didn't happen here that I should be aware of.)
Yes. I was considering it as a follow-up, but I wanted to do the simplest thing for the initial version since it already had enough chance of breaking things or landing with conflicts.
Feel free to file a follow-up.
Description
•