Stop using pinned atoms in a few places
Categories
(Core :: JavaScript Engine, task, P2)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox97 | --- | fixed |
People
(Reporter: jandem, Assigned: jandem)
References
Details
Attachments
(6 files)
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review |
There are some places where we use pinned atoms that we can change to non-pinned atoms. This improves memory usage and fixes part of the regression in bug 1743423.
|
Assignee | |
Comment 1•2 years ago
|
||
|
|
Assignee | |
Comment 2•2 years ago
|
||
Depends on D132688
|
|
Assignee | |
Comment 3•2 years ago
|
||
This also implements a copy constructor for XPCNativeMember to balance ctor/dtor counts.
Depends on D132689
|
|
Assignee | |
Comment 4•2 years ago
|
||
This patch ensures that tracing XPCNativeInterface through the map in XPCJSRuntime
(implemented in a later patch) is sufficient to trace every live XPCNativeInterface.
Depends on D132690
|
|
Assignee | |
Comment 5•2 years ago
|
||
Pinned atoms are marked implicitly, so if we stop pinning these atoms we need to
mark them explicitly for the current zone.
Depends on D132691
|
|
Assignee | |
Comment 6•2 years ago
|
||
Replace the use of pinned atoms with code to trace all atoms stored in live
XPCNativeInterface instances. This means we stop 'leaking' those atoms after
releasing the XPCNativeInterface using them.
Depends on D132692
|
||
Comment 7•2 years ago
|
||
Some random food for thought: these jsids are the only parts of XPCNativeInterface and XPCNativeMember that are dynamically allocated, so if there was some way to compile those into the binary I think we could statically generate IID2NativeInterfaceMap and not use any dynamic memory for it.
|
|
Assignee | |
Comment 8•2 years ago
|
||
(In reply to Andrew McCreight [:mccr8] from comment #7)
Some random food for thought: these jsids are the only parts of XPCNativeInterface and XPCNativeMember that are dynamically allocated, so if there was some way to compile those into the binary I think we could statically generate IID2NativeInterfaceMap and not use any dynamic memory for it.
Yeah, that would be ideal. We've talked a few times about replacing pinned atoms with statically allocated atoms/jsids, probably worth investigating more.
Pushed by jdemooij@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9553a8b296a6 part 1 - Don't use a pinned atom in ReportCantConvert. r=jonco https://hg.mozilla.org/integration/autoland/rev/39ce51389a5c part 2 - Don't use pinned atoms in a few places in DOM bindings. r=jonco,peterv https://hg.mozilla.org/integration/autoland/rev/2a2dbb384a2b part 3 - Use a Vector for the list of members in XPCNativeInterface::NewInstance. r=mccr8 https://hg.mozilla.org/integration/autoland/rev/9441b1f8bd3e part 4 - Add the new XPCNativeInterface to the map in XPCNativeInterface::NewInstance. r=mccr8,jonco https://hg.mozilla.org/integration/autoland/rev/a5fe1d14598a part 5 - Fix atom marking for XPCNativeMember names. r=mccr8,jonco https://hg.mozilla.org/integration/autoland/rev/b40a7a2cf1c8 part 6 - Stop pinning atoms for XPCNativeInterface. r=mccr8,jonco
|
||
Comment 10•2 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/9553a8b296a6
https://hg.mozilla.org/mozilla-central/rev/39ce51389a5c
https://hg.mozilla.org/mozilla-central/rev/2a2dbb384a2b
https://hg.mozilla.org/mozilla-central/rev/9441b1f8bd3e
https://hg.mozilla.org/mozilla-central/rev/a5fe1d14598a
https://hg.mozilla.org/mozilla-central/rev/b40a7a2cf1c8
|
||
Comment 11•2 years ago
|
||
== Change summary for alert #32664 (as of Thu, 09 Dec 2021 19:20:24 GMT) ==
Improvements:
| Ratio | Test | Platform | Options | Absolute values (old vs new) |
|---|---|---|---|---|
| 1% | Base Content JS | linux1804-64-shippable-qr | 1,828,142.33 -> 1,818,424.00 | |
| 1% | Base Content JS | windows10-64-2004-shippable-qr | 1,833,592.00 -> 1,823,864.00 | |
| 1% | Base Content JS | macosx1015-64-shippable-qr | 1,860,424.00 -> 1,850,696.00 |
For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=32664
Description
•