1102820 - Clean up static constructor calls introduced by bug 1073096

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Lars T Hansen [:lth]]

After landing bug 1073096 I received an anoymous letter that is threatening yet vague (indeed I initially thought "Constructors" referred to JS constructor functions):

-----8<-----------------------------------------------------------------

Regression: Mozilla-Inbound - Number of Constructors - CentOS release 5 (Final) - 16% increase
----------------------------------------------------------------------------------------------
    Previous: avg 72.000 stddev 0.000 of 12 runs up to revision 8d3f59b2b614
    New     : avg 83.500 stddev 0.905 of 12 runs since revision e3cba62ada40
    Change  : +11.500 (16% / z=0.000)
    Graph   : http://mzl.la/1ufYB3E

Changeset range: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=8d3f59b2b614&tochange=e3cba62ada40

Changesets:
  * http://hg.mozilla.org/integration/mozilla-inbound/rev/4e9c799bccc6
    : Lars T Hansen <[e-mail]> - Bug 1073096 - Use MemoryBarrierBits type as container for bit sets. r=luke
    : http://bugzilla.mozilla.org/show_bug.cgi?id=1073096

  * http://hg.mozilla.org/integration/mozilla-inbound/rev/d64f299df337
    : Lars T Hansen <[e-mail]> - Bug 1073096 - Support for Odin and asm.js.  r=luke
    : http://bugzilla.mozilla.org/show_bug.cgi?id=1073096

  * http://hg.mozilla.org/integration/mozilla-inbound/rev/e3cba62ada40
    : Lars T Hansen <[e-mail]> - Bug 1073096 - ARM support for atomics for Odin/asm.js.  r=dtc-moz
    : http://bugzilla.mozilla.org/show_bug.cgi?id=1073096

Bugs:
  * http://bugzilla.mozilla.org/show_bug.cgi?id=1073096 - Atomics and locks for SharedArrayBuffer: asm.js

----- 8< --------------------------------------------------------------------

Ms2ger suggests that the cause of this is the first of these patches, which defines some global constants in terms of constructor calls.  I naively expected those to be resolved by the compiler; they are not.

Ms2ger also suggests MOZ_CONSTEXPR (see eg bug 968942) could be brought to bear on the problem.

Comment 1

[Lars T Hansen [:lth]]

Attachment: Probable fix

This patch compiles on Linux-64 but I've not tested it more broadly, nor figured out how to show that it fixes the problem.

Comment 2

[Lars T Hansen [:lth]]

Argument that this thing works as it should: grepping the object files of a release build for symbols that represent variables whose values have to be constructed.

Without patch:

$ nm -A *.o | grep Membar
jsarray.o:0000000000000004 b _ZN2js3jitL10MembarFullE
jsarray.o:0000000000000000 b _ZN2js3jitL15MembarAfterLoadE
jsatom.o:0000000000000004 b _ZN2js3jitL10MembarFullE
jsatom.o:0000000000000000 b _ZN2js3jitL15MembarAfterLoadE
jsmath.o:0000000000000004 b _ZN2js3jitL10MembarFullE
jsmath.o:0000000000000000 b _ZN2js3jitL15MembarAfterLoadE
Parser.o:0000000000000004 b _ZN2js3jitL10MembarFullE
Parser.o:0000000000000000 b _ZN2js3jitL15MembarAfterLoadE
RegExp.o:0000000000000004 b _ZN2js3jitL10MembarFullE
RegExp.o:0000000000000000 b _ZN2js3jitL15MembarAfterLoadE
Unified_cpp_js_src0.o:00000000000066dc b _ZN2js3jitL10MembarFullE
Unified_cpp_js_src0.o:00000000000066d8 b _ZN2js3jitL15MembarAfterLoadE
Unified_cpp_js_src10.o:00000000000001c4 b _ZN2js3jitL10MembarFullE
Unified_cpp_js_src10.o:00000000000001c0 b _ZN2js3jitL15MembarAfterLoadE
Unified_cpp_js_src11.o:0000000000000080 b _ZN2js3jitL10MembarFullE
Unified_cpp_js_src11.o:000000000000007c b _ZN2js3jitL15MembarAfterLoadE
Unified_cpp_js_src12.o:000000000000006c b _ZN2js3jitL10MembarFullE
Unified_cpp_js_src12.o:0000000000000068 b _ZN2js3jitL15MembarAfterLoadE
Unified_cpp_js_src1.o:000000000000001c b _ZN2js3jitL10MembarFullE
Unified_cpp_js_src1.o:0000000000000018 b _ZN2js3jitL15MembarAfterLoadE
Unified_cpp_js_src2.o:0000000000000004 b _ZN2js3jitL10MembarFullE
Unified_cpp_js_src2.o:0000000000000000 b _ZN2js3jitL15MembarAfterLoadE
Unified_cpp_js_src3.o:0000000000003a04 b _ZN2js3jitL10MembarFullE
Unified_cpp_js_src3.o:0000000000003a00 b _ZN2js3jitL15MembarAfterLoadE
Unified_cpp_js_src4.o:00000000000000d4 b _ZN2js3jitL10MembarFullE
Unified_cpp_js_src4.o:00000000000000d0 b _ZN2js3jitL15MembarAfterLoadE
Unified_cpp_js_src5.o:0000000000000004 b _ZN2js3jitL10MembarFullE
Unified_cpp_js_src5.o:0000000000000000 b _ZN2js3jitL15MembarAfterLoadE
Unified_cpp_js_src6.o:00000000000000ac b _ZN2js3jitL10MembarFullE
Unified_cpp_js_src6.o:00000000000000a8 b _ZN2js3jitL15MembarAfterLoadE
Unified_cpp_js_src7.o:0000000000000084 b _ZN2js3jitL10MembarFullE
Unified_cpp_js_src7.o:0000000000000080 b _ZN2js3jitL15MembarAfterLoadE
Unified_cpp_js_src8.o:0000000000000010 b _ZN2js3jitL10MembarFullE
Unified_cpp_js_src8.o:000000000000000c b _ZN2js3jitL15MembarAfterLoadE
Unified_cpp_js_src9.o:000000000000001c b _ZN2js3jitL10MembarFullE
Unified_cpp_js_src9.o:0000000000000018 b _ZN2js3jitL15MembarAfterLoadE

With patch:

$ nm -A *.o | grep Membar
(nothing)

Comment 3

[Lars T Hansen [:lth]]

The try run is green except that some builds error out for unused functions, sigh:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=148f5b503e6d

Comment 4

[Lars T Hansen [:lth]]

Making the functions inline fixes the compile errors:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=99e7e138eb4a

Comment 5

[Lars T Hansen [:lth]]

Attachment: Introduce MOZ_CONSTEXPR and MOZ_CONSTEXPR_VAR

Mike, somebody suggested you'd be a person to know about this sort of thing (even though not a peer on the JS engine, I think).

Comment 6

[Lars T Hansen [:lth]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/76097ee6d6f9

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/76097ee6d6f9