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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: froydnj, Unassigned)

References

Details

Attachments

(1 file)

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.
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?
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....
Flags: needinfo?(nfroyd)
Blocks: 909338
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)
Flags: needinfo?(nfroyd)
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+
Sanity-checking on Try with a big fat comment: https://tbpl.mozilla.org/?tree=Try&rev=df023987208d
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-
https://hg.mozilla.org/mozilla-central/rev/df525a547cf2
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: