948638 - Always use jsid struct types

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Sean Stangl [:sstangl]]

Attachment: 0001-Use-jsid-struct-types-in-opt-builds.patch

Bug 939505 introduced JS_DEBUG for exported headers in order to allow linking against ptrdiff_t jsid mozjs when DEBUG is defined for the parent project. Bug 948099 extends JS_DEBUG to the rest of the engine.

Waldo pointed out that we wouldn't need JS_DEBUG for jsid if we always used jsid struct types, and that we haven't measured the performance impact of making that switch.

The attached patch makes that change. Landing this patch will hopefully let us revert JS_DEBUG.

Octane performance is largely unchanged, except that octane-regexp receives a consistent performance improvement from ~1800 to ~1950 on both x86 and x86_64. I'm not sure why that is, but I'll take it.

Comment 1

[Jeff Walden [:Waldo]]

Comment on attachment 8345502 [details] [diff] [review]
0001-Use-jsid-struct-types-in-opt-builds.patch

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

::: js/public/Id.h
@@ +33,5 @@
>      size_t asBits;
>      bool operator==(jsid rhs) const { return asBits == rhs.asBits; }
>      bool operator!=(jsid rhs) const { return asBits != rhs.asBits; }
>  };
> +#define JSID_BITS(id) (id.asBits)

Would be nice to get rid of this too, but that can be done in a separate patch, no need to hold this up for that.

Comment 2

[Nathan Froyd [:froydnj]]

Make sure to double check that this patch doesn't increase the number of static constructors; you might have to sprinkle some MOZ_CONSTEXPR about.

Comment 3

[Sean Stangl [:sstangl]]

Per a Try run at https://tbpl.mozilla.org/?tree=Try&rev=23834f5dc1d0, num_ctors increased from 94 to 123. Looking at the Tryserver history at http://graphs.mozilla.org/graph.html#tests=[[81,23,6]]&sel=none&displayrange=7&datatype=running shows this 123 range occurring intermittently.

Comment 4

[Sean Stangl [:sstangl]]

Pushed. Will address possible static constructor count increase after some runs.

https://hg.mozilla.org/integration/mozilla-inbound/rev/a9acb6b5ed0f

Comment 5

[Mike Hommey [:glandium]]

Regression: Mozilla-Inbound - Number of Constructors - CentOS (x86_64) release 5 (Final) - 35.1% increase
---------------------------------------------------------------------------------------------------------
    Previous: avg 94.000 stddev 0.000 of 12 runs up to revision 28587965e647
    New     : avg 127.000 stddev 0.000 of 12 runs since revision a9acb6b5ed0f
    Change  : +33.000 (35.1% / z=0.000)
    Graph   : http://mzl.la/1f5TRZu

Changeset range: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=28587965e647&tochange=a9acb6b5ed0f

Changesets:
  * http://hg.mozilla.org/integration/mozilla-inbound/rev/a9acb6b5ed0f
    : Sean Stangl <[e-mail]> - Bug 948638 - Always use jsid struct types. r=Waldo
    : http://bugzilla.mozilla.org/show_bug.cgi?id=948638

Bugs:
  * http://bugzilla.mozilla.org/show_bug.cgi?id=948638 - Always use jsid struct types

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a9acb6b5ed0f