Closed Bug 913640 Opened 6 years ago Closed 6 years ago

Regression: increase in the number of static constructors in bug 910322

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: kats, Assigned: froydnj)

References

Details

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #910322 +++

The build cruncher slapped my hand.

Regression: B2g-Inbound - Number of Constructors - CentOS (x86_64) release 5 (Final) - 3.08% increase
-----------------------------------------------------------------------------------------------------
    Previous: avg 130.000 stddev 0.000 of 12 runs up to revision d4a48a629fbe
    New     : avg 134.000 stddev 0.000 of 12 runs since revision 610908ca3739
    Change  : +4.000 (3.08% / z=0.000)
    Graph   : http://mzl.la/19q25sn

Apparently some constexpr magic should fix this. Running it through try now: https://tbpl.mozilla.org/?tree=Try&rev=e9d4d77e616a
Unfortunately the above try run did not decrease the number of static constructors relative to the baseline (m-c cset b818c7222968) - both show the number of constructors as 121. The actual patch I tried is at https://hg.mozilla.org/try/rev/1045d7844b60
constexpr seems to work for me locally, are you just not including mozilla/Attributes.h and so MOZ_CONSEXPR evaluates to nothing?
If Attributes.h wasn't getting included it would throw a compile error, no? Regardless I pushed to try with the include and it didn't make any difference: https://tbpl.mozilla.org/?tree=Try&rev=f81fb2ed73fc

If you have a patch that works locally, can you push it?
I pushed https://tbpl.mozilla.org/?tree=Try&rev=b2add3ee84bc which locally takes me from 108 to 103 static constructors
Thanks. Unfortunately this doesn't appear to reduce the number of constructors on tbpl. Baseline [1] and try [2] both have 121 constructors.

[1] https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=050debae43a6
[2] https://tbpl.mozilla.org/?tree=Try&rev=b2add3ee84bc
That's pretty weird, I've never seen one version of gcc emit static constructors where another wouldn't, but I'll test with 4.7 locally then
(In reply to Trevor Saunders (:tbsaunde) from comment #7)
> That's pretty weird, I've never seen one version of gcc emit static
> constructors where another wouldn't, but I'll test with 4.7 locally then

4.7 appears to inline the uses of kViewport*, at least on my x86-64 linux box.  It's possible that talos fingered the wrong person, though kats's push does seem to satisfy "did these things cause static constructors".

Check the static constructors for content/base/src/nsDocument.o; you shouldn't see any for kViewport*.
(In reply to Nathan Froyd (:froydnj) from comment #8)
> (In reply to Trevor Saunders (:tbsaunde) from comment #7)
> > That's pretty weird, I've never seen one version of gcc emit static
> > constructors where another wouldn't, but I'll test with 4.7 locally then
> 
> 4.7 appears to inline the uses of kViewport*, at least on my x86-64 linux
> box.  It's possible that talos fingered the wrong person, though kats's push
> does seem to satisfy "did these things cause static constructors".
> 
> Check the static constructors for content/base/src/nsDocument.o; you
> shouldn't see any for kViewport*.

Never mind, I do see these in my build now.  Haven't checked to see whether MOZ_CONSTEXPR makes them go away.
Ah, duh.  The CSSIntSize / SizeTyped bits need MOZ_CONSTEXPR too.  Preparing a patch for that.
This patch removes the static constructors for kViewport* from the couple of
object files I checked; full build currently running to verify that it
removes them from libxul entirely.
Attachment #804528 - Flags: review?(bugmail.mozilla)
yeah, inlining is a sane explanation of what's happening here (I was looking at nsViewPortInfo.o which which)
Comment on attachment 804528 [details] [diff] [review]
sprinkle MOZ_CONSTEXPR on gfx's typed units to reduce static constructors

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

LGTM, thanks!
Attachment #804528 - Flags: review?(bugmail.mozilla) → review+
Thanks!
Assignee: bugmail.mozilla → nfroyd
https://hg.mozilla.org/mozilla-central/rev/fb9df2cda822
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.