Closed Bug 1102820 Opened 10 years ago Closed 9 years ago

Clean up static constructor calls introduced by bug 1073096

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: lth, Assigned: lth)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

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 <lhansen@mozilla.com> - 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 <lhansen@mozilla.com> - 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 <lhansen@mozilla.com> - 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.
Attached patch Probable fix (obsolete) — Splinter Review
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.
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)
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
Making the functions inline fixes the compile errors:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=99e7e138eb4a
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).
Attachment #8526663 - Attachment is obsolete: true
Attachment #8526859 - Flags: review?(mh+mozilla)
Attachment #8526859 - Flags: review?(mh+mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/76097ee6d6f9
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.