974360 - Fix the static constructors added by bug 910754

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[(no longer active)]

Regression: Mozilla-Inbound - Number of Constructors - CentOS release 5 (Final) - 29.9% increase
------------------------------------------------------------------------------------------------
    Previous: avg 67.000 stddev 0.000 of 12 runs up to revision 09ab3f8a5695
    New     : avg 87.000 stddev 0.000 of 12 runs since revision f8cd1f2d686e
    Change  : +20.000 (29.9% / z=0.000)
    Graph   : http://mzl.la/1m8Fs2i

Changeset range: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=09ab3f8a5695&tochange=f8cd1f2d686e

Changesets:
  * http://hg.mozilla.org/integration/mozilla-inbound/rev/f6c2efda0788
    : George Wright <[e-mail]> - Bug 910754 - Update Skia to r13424 r=upstream
    : http://bugzilla.mozilla.org/show_bug.cgi?id=910754

  * http://hg.mozilla.org/integration/mozilla-inbound/rev/5d933587c726
    : George Wright <[e-mail]> - Bug 910754 - Add a bunch more required OpenGL functions for SkiaGL when using desktop OpenGL r=vlad
    : http://bugzilla.mozilla.org/show_bug.cgi?id=910754

  * http://hg.mozilla.org/integration/mozilla-inbound/rev/0c3c16ea5cd3
    : George Wright <[e-mail]> - Bug 910754 - skia-npapi's anp_getFontPath() function doesn't work with upstream Skia anymore, and it looks like we don't need it to be implemented anyway r=snorp
    : http://bugzilla.mozilla.org/show_bug.cgi?id=910754

  * http://hg.mozilla.org/integration/mozilla-inbound/rev/56474b57c3b8
    : George Wright <[e-mail]> - Bug 910754 - Update gfx/2d's Skia code to use the new Skia APIs r=snorp
    : http://bugzilla.mozilla.org/show_bug.cgi?id=910754

  * http://hg.mozilla.org/integration/mozilla-inbound/rev/218aa532182b
    : George Wright <[e-mail]> - Bug 910754 - Stub out CreateTypeface*() for Mac r=snorp
    : http://bugzilla.mozilla.org/show_bug.cgi?id=910754

  * http://hg.mozilla.org/integration/mozilla-inbound/rev/87859a17d799
    : George Wright <[e-mail]> - Bug 910754 - Update include paths for gonk's build as Skia's directory structure has changed r=snorp
    : http://bugzilla.mozilla.org/show_bug.cgi?id=910754

  * http://hg.mozilla.org/integration/mozilla-inbound/rev/07dc7e2ed993
    : George Wright <[e-mail]> - Bug 910754 - Mark linear-gradient-1a and linear-gradient-1b as fails-if with Skia
    : http://bugzilla.mozilla.org/show_bug.cgi?id=910754

  * http://hg.mozilla.org/integration/mozilla-inbound/rev/5498f482b7ff
    : George Wright <[e-mail]> - Bug 910754 - [Skia] Defer deletion of our shader objects until after linking the program r=upstream(bsalomon)
    : http://bugzilla.mozilla.org/show_bug.cgi?id=910754

  * http://hg.mozilla.org/integration/mozilla-inbound/rev/29a97babd395
    : George Wright <[e-mail]> - Bug 910754 - Update SkFontHost_cairo for the new Skia APIs r=snorp
    : http://bugzilla.mozilla.org/show_bug.cgi?id=910754

  * http://hg.mozilla.org/integration/mozilla-inbound/rev/609d3ba60b5b
    : George Wright <[e-mail]> - Bug 910754 - Update SkUserConfig to cater for SkMutex/SkAtomics header locations r=snorp
    : http://bugzilla.mozilla.org/show_bug.cgi?id=910754

  * http://hg.mozilla.org/integration/mozilla-inbound/rev/602341e2f084
    : George Wright <[e-mail]> - Bug 910754 - Update include location for skia-npapi to look in skia/trunk r=snorp
    : http://bugzilla.mozilla.org/show_bug.cgi?id=910754

  * http://hg.mozilla.org/integration/mozilla-inbound/rev/14e857f528b2
    : George Wright <[e-mail]> - Bug 910754 - As we never build Skia statically, disable Skia's instance counting as it requires being built as a static library. r=snorp
    : http://bugzilla.mozilla.org/show_bug.cgi?id=910754

  * http://hg.mozilla.org/integration/mozilla-inbound/rev/29fcbc4e91f3
    : George Wright <[e-mail]> - Bug 910754 - Re-apply bug 921670 - Import the old Android FontHost to provide fonts for skia-npapi r=snorp
    : http://bugzilla.mozilla.org/show_bug.cgi?id=910754

  * http://hg.mozilla.org/integration/mozilla-inbound/rev/c53990676a37
    : George Wright <[e-mail]> - Bug 910754 - Ensure B2G builds don't use SkFontMgr r=snorp
    : http://bugzilla.mozilla.org/show_bug.cgi?id=910754

  * http://hg.mozilla.org/integration/mozilla-inbound/rev/f8cd1f2d686e
    : George Wright <[e-mail]> - Bug 910754 - Update CLOBBER for the Skia update
    : http://bugzilla.mozilla.org/show_bug.cgi?id=910754

Bugs:
  * http://bugzilla.mozilla.org/show_bug.cgi?id=910754 - [Skia] Update Skia, 2014Q1

Comment 1

[Nathan Froyd [:froydnj]]

I haven't looked closely at the changes, but Skia is one of the biggest remaining sources of static constructors; it wouldn't really surprise me if updating Skia dragged in a bunch of new static constructors.  Skia's static constructors would require some moderately significant changes to eliminate. :(

Comment 2

[Andreas Gal :gal]

Static constructors are **** in many ways and I get why we don't like them in gecko code, but do we care about static constructors in 3rd party code enough? If this impacts startup, its probably feasible to load skia dynamically only once we need it (note: everywhere except windows we expect skia to be the only way we paint at some point, so eventually this would be on the startup path anyway, windows aside).

Comment 3

[Nathan Froyd [:froydnj]]

(In reply to Andreas Gal :gal from comment #2)
> Static constructors are shitty in many ways and I get why we don't like them
> in gecko code, but do we care about static constructors in 3rd party code
> enough?

We don't have any good way of distinguishing between the two; if the code is in libxul, it counts as a static constructor, regardless of which bit of code it's from.

Occasionally, there are options in third party libraries for decreasing static constructors; skia has a compile option that makes static mutexes on Linux/Android not generate static constructors, for instance.

Comment 4

[George Wright (:gw280) (needinfo me!)]

Attachment: static_global

This will probably help a little bit. Currently green on graphics: https://tbpl.mozilla.org/?tree=Graphics

Comment 5

[George Wright (:gw280) (needinfo me!)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/361cea233edd

(Vlad gave r+ on #gfx)

Comment 6

[George Wright (:gw280) (needinfo me!)]

Improvement: Mozilla-Inbound - Number of Constructors - CentOS release 5 (Final) - 33.3% decrease
-------------------------------------------------------------------------------------------------
    Previous: avg 87.000 stddev 0.000 of 12 runs up to revision 8f1c76b3a931
    New     : avg 58.000 stddev 0.000 of 12 runs since revision 361cea233edd
    Change  : -29.000 (33.3% / z=0.000)
    Graph   : http://mzl.la/1mbKJX2

Resolving fixed as we're now actually lower than we were before the Skia update landed \o/

Comment 7

[George Wright (:gw280) (needinfo me!)]

(Actually, going to leave it open because I just noticed it's not hit m-c yet). Please close when this patch has been merged into m-c.

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/361cea233edd