Convert RemoteController.js back into a jsm
Categories
(Toolkit :: XUL Widgets, defect, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox67 | --- | affected |
People
(Reporter: Gijs, Unassigned)
References
Details
(Whiteboard: [fxperf:p3])
The way bug 1395795 was fixed means we now re-load and re-evaluate RemoteController.js for every browser in a window. In my current window, that means we've loaded and evaluated it 101 times. That's not very nice. We should come up with a better way.
Ehsan, can you expand on what bug 1395795 was trying to accomplish? From the scant comments on the bug, I don't really understand what wrappers it's talking about, why that generates jank, and if there are alternatives that don't have the downsides the current solution does.
Reporter | ||
Comment 1•6 years ago
|
||
(In reply to :Gijs (he/him) from comment #0)
Ehsan, can you expand on what bug 1395795 was trying to accomplish? From the scant comments on the bug, I don't really understand what wrappers it's talking about, why that generates jank, and if there are alternatives that don't have the downsides the current solution does.
Actually needinfo'ing this time.
Updated•6 years ago
|
Comment 2•6 years ago
|
||
Sure, see this profile https://perfht.ml/2CfRCXh from bug 1395274.
The cost being discussed here was the cost of wrapping the arguments passed to enableDisableCommands when wrapping them inside a CCW as you can see in that profile. By moving the code for that function outside of a JSM, we made sure the chrome code on the caller and callee frames run in the same compartment and therefore no CCW would be necessary for the call.
I believe that the compartment setup for chrome code right now has changed drastically since that time, though I haven't followed all of the details, so putting this code back in a JSM without incurring this regression might be viable now...
Reporter | ||
Comment 3•6 years ago
|
||
(In reply to :Ehsan Akhgari from comment #2)
I believe that the compartment setup for chrome code right now has changed drastically since that time, though I haven't followed all of the details, so putting this code back in a JSM without incurring this regression might be viable now...
Who would know whether that would make sense?
The other option here seems to be to make the thing more uniquely named and load it once per chrome window instead of once per browser, which would also mostly fix this issue.
Updated•6 years ago
|
Comment 4•6 years ago
|
||
(In reply to :Gijs (he/him) from comment #3)
(In reply to :Ehsan Akhgari from comment #2)
I believe that the compartment setup for chrome code right now has changed drastically since that time, though I haven't followed all of the details, so putting this code back in a JSM without incurring this regression might be viable now...
Who would know whether that would make sense?
I believe this is now fixed given that bug 1512029 and bug 1514210 are both fixed, but jandem would know best if we can still have cases where chrome code can run in more than one system principal compartment?
Comment 5•6 years ago
|
||
(In reply to :Ehsan Akhgari from comment #4)
I believe this is now fixed given that bug 1512029 and bug 1514210 are both fixed, but jandem would know best if we can still have cases where chrome code can run in more than one system principal compartment?
Yes chrome code running in windows, sandboxes, JSMs should all be same-compartment now so I think moving it back into a JSM will be fine :)
Reporter | ||
Updated•6 years ago
|
Comment 6•5 years ago
|
||
As far as I see, this was fixed in bug 1604003.
Please reopen if there is still more to do here.
Bug 1558520 will change more of this stuff.
Description
•