Closed
Bug 909328
Opened 11 years ago
Closed 11 years ago
308% increase in the number of static constructors
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla26
People
(Reporter: froydnj, Unassigned)
References
Details
Attachments
(1 file)
2.95 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
Pushlog for the regression is: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=862513e70ed3&tochange=18311b19cd53 After scanning through the patches, I think bug 908576 is the likeliest candidate. Probably just have to sprinkle some constexpr about or move statics to function-level instead of file-leve.
Comment 1•11 years ago
|
||
Hmm. The most likely issue here, I suspect, is http://hg.mozilla.org/integration/mozilla-inbound/rev/fd3c54248f94 It changed he codegen from having this in the header: extern const NativePropertyHooks sNativePropertyHooks; and the struct definition in the .cpp, to having this in the header: extern const NativePropertyHooks* sNativePropertyHooks; and then the .cpp has the struct definition followed by: const NativePropertyHooks* sNativePropertyHooks = &sNativePropertyHooksStruct; The goal was to be able to forward-declare NativePropertyHooks in the header. Is there a saner way we can do that?
Comment 2•11 years ago
|
||
That's assuming that moving structs from extern to static won't suddenly create new constructors, of course, which I assume is the case. I suppose we could change from exposing a sNativePropertyHooks to exposing a function that returns the pointer....
Updated•11 years ago
|
Flags: needinfo?(nfroyd)
Reporter | ||
Comment 3•11 years ago
|
||
This patch is sort of ugly, but it does make the constructors go away on Android. I am not entirely sure *why* it works; I think GCC sees the pointer as not compile-time invariant and so refuses to fold constructors to constants. Whereas with the arrays (and the previous code, which used |&sNativePropertyHookStructThing|, GCC can see through the address arithmetic...somehow?
Attachment #795487 -
Flags: review?(bzbarsky)
Reporter | ||
Updated•11 years ago
|
Flags: needinfo?(nfroyd)
Comment 4•11 years ago
|
||
Comment on attachment 795487 [details] [diff] [review] coerce GCC into not generating static constructors for WebIDL property hooks Ugh, but r=me. Please add a comment explaining why the setup is what it is?
Attachment #795487 -
Flags: review?(bzbarsky) → review+
Reporter | ||
Comment 5•11 years ago
|
||
Sanity-checking on Try with a big fat comment: https://tbpl.mozilla.org/?tree=Try&rev=df023987208d
Reporter | ||
Comment 6•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/df525a547cf2 Tested compilation locally and on limited number of try platforms: https://tbpl.mozilla.org/?tree=Try&rev=df023987208d
Flags: in-testsuite-
Comment 7•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/df525a547cf2
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Assignee | ||
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•