Closed
Bug 1385384
Opened 7 years ago
Closed 7 years ago
Don't store mPlaceholderTransactionWeak as a weak pointer
Categories
(Core :: DOM: Editor, enhancement)
Core
DOM: Editor
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
8.32 KB,
patch
|
masayuki
:
review+
|
Details | Diff | Splinter Review |
Weak pointers are needlessly expensive, and they show up in profiles, for example see https://perfht.ml/2v75r7A. The reason why this used to be a weak pointer was that this code goes back to before we had a cycle collector, but with a cycle collector this doesn't need to be a weak pointer any more at all, and can just be a normal strong pointer. This way we can avoid the extra cost of dynamically allocating the proxy object.
Assignee | ||
Comment 1•7 years ago
|
||
Attachment #8891451 -
Flags: review?(masayuki)
Assignee | ||
Comment 2•7 years ago
|
||
This patch alone shaves off a couple of milliseconds from the test case of bug 1346723 for me locally.
Comment 3•7 years ago
|
||
Comment on attachment 8891451 [details] [diff] [review] Don't store mPlaceholderTransactionWeak as a weak pointer Interesting. I don't understand about Cycle Collector, though, how about other weakptr members like mSelectionControllerWeak and mDocumentWeak? If they don't need to be weak pointers, we can reduce the runtime cost for them too.
Attachment #8891451 -
Flags: review?(masayuki) → review+
Assignee | ||
Comment 4•7 years ago
|
||
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #3) > Comment on attachment 8891451 [details] [diff] [review] > Don't store mPlaceholderTransactionWeak as a weak pointer > > Interesting. I don't understand about Cycle Collector, though, how about > other weakptr members like mSelectionControllerWeak and mDocumentWeak? If > they don't need to be weak pointers, we can reduce the runtime cost for them > too. Yes, those can probably be converted into strong pointers in a very similar way. The cost of mSelectionControllerWeak definitely shows up through GetSelection() in profiles. I may get to those later...
Pushed by eakhgari@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/780d578d1f74 Don't store mPlaceholderTransactionWeak as a weak pointer; r=masayuki
Comment 6•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/780d578d1f74
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•