Closed Bug 1027425 Opened 5 years ago Closed 5 years ago

Clean up static initializers for proxy handler instances

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: efaust, Assigned: efaust)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

We have all these singleton instances of C++ Proxy Handlers laying around, and calling static initializers for them is silly, and costly for b2g, as I understand it.

Some months ago, I told Nathan he was gonna ruin a patch of mine if he went forward with fixing this, and that I would take care of it. Now that we're doing bug 1027402, this should be pretty straight forward
Depends on: 1027964
Blocks: 966518
Attached patch Patch (obsolete) — Splinter Review
Finally do what I promised forever ago. I will push this stack to try as soon as it reopens and post here.

r? froydnj to ensure someone who knows all the ins and outs takes a look at this.
Assignee: nobody → efaustbmo
Status: NEW → ASSIGNED
Attachment #8477080 - Flags: review?(nfroyd)
Attachment #8477080 - Flags: review?(bzbarsky)
Depends on: 1017862
Attached patch Patch v2Splinter Review
Fix some failures seen on try after I was finally able to push. This patch, as well as those from the two open blockers on try at https://tbpl.mozilla.org/?tree=Try&rev=d6dee568944f
Attachment #8477080 - Attachment is obsolete: true
Attachment #8477080 - Flags: review?(nfroyd)
Attachment #8477080 - Flags: review?(bzbarsky)
Attachment #8477812 - Flags: review?(nfroyd)
Attachment #8477812 - Flags: review?(bzbarsky)
Comment on attachment 8477812 [details] [diff] [review]
Patch v2

Should jswrapper really be using aFoo arg names?

The WaiveXrayWrapper ctor should be explicit, I'd hope.

Why did a bunch of the Xray helper classes move to the header?  Is it just so we can make XrayWrapper have a constexpr ctor?  That seems really annoying.  :(  Can we just leave it be?

r=me on the rest, but I'd like to understand what the deal is with the Xray changes.
Attachment #8477812 - Flags: review?(bzbarsky) → review+
Comment on attachment 8477812 [details] [diff] [review]
Patch v2

Review of attachment 8477812 [details] [diff] [review]:
-----------------------------------------------------------------

Worth explicitly noting that this won't help B2G until we upgrade the (Gecko) compiler for B2G, since GCC < 4.6 doesn't support constexpr...but at least everybody else benefits in the interim!
Attachment #8477812 - Flags: review?(nfroyd) → review+
https://hg.mozilla.org/mozilla-central/rev/f41c1edabfe8
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.