579471 - JM: Special-case |new Array(...)| and other native constructors

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[David Mandelin [:dmandelin]]

Make this fast:

    function f() {
      var a;
      for (var i = 0; i < 42864; ++i) {
        a = new Array();
      }
    }

    var t0 = new Date;
    f();
    print(new Date - t0);


We create 42864 arrays in SunSpider. See TraceRecoder::newArray for design ideas.

JM wanted SS win: 22 ms

Comment 1

[Brian Hackett [Laid off!]]

Attachment: fast ctor patch

Overhead here is split between the slow call done for each 'new' (fetch the prototype property, make a new stack frame, etc.) and the overhead of js_Array itself.

Patch from last week attached (crude, breaks other stuff) which lets constructors be fast natives and fixes the first category (this also applies to other commonly called 'new' natives, like Object/Date).  When the ctor is called with 'new' then 'this' is a magic value, and the native makes the needed object.

Comment 2

[Brian Hackett [Laid off!]]

Attachment: revised fast ctor patch

This patch does a few things:

Adds a JSCLASS_FAST_CONSTRUCTOR flag to JSAPI which allows the function passed to JS_InitClass to be a FastNative.

Convert Array/Object/String/Date constructors to fast natives (the only ones hit on SS in any abundance).

Modifies InvokeConstructor and SlowNew in the methodjit to call fast natives directly, without constructing a stack frame or new object first.  Instead, the 'this' value is marked as magic, and the native must either see that and fill in the new object (much faster than InvokeConstructor would, skipping the prototype lookup etc.) or call ComputeThisFromVp (as it normally would), making the new object on demand.  In all reasonable cases the former path is taken; the latter is there for weird stuff like 'x = new y.charAt(z)'.

This saves 11.8ms on SS for me, split between 3d-raytrace, crypto-aes, date-format-tofte and date-format-xparb.  There are still big gains to get in object creation, mainly in fixing GC and page fault overhead.

Comment 3

[Brian Hackett [Laid off!]]

Attachment: non-breaking fast ctor patch

This patch uses a low bit in JSFunction.flags to distinguish fast natives which can or can't be called using a magic 'this' value with no constructed object.  Doesn't affect the existing API.

Comment 4

[Brendan Eich [:brendan]]

See bug 577648's patch (attachment 460169 [details] [diff] [review]), the first jsfun.h hunk, where I define a JSFUN_JOINABLE (0x01). I don't think JSFUN_FAST_CTOR should be in the public jsapi.h (also, name nit: JSFUN_FAST_NATIVE_CTOR seems better).

/be

Comment 5

[Brian Hackett [Laid off!]]

Attachment: Rebased to current JM

Re: comment 4, whether the JSFUN_FAST_CTOR and JSCLASS_FAST_CONSTRUCTOR are in jsapi.h depends on whether magic values are considered to be part of the API.  With this patch, one can't write a fast native (whether a class constructor or not) that behaves special when invoked with 'new' if values can't be tested for magic.  Which headers form the API?  There's no JSVAL_IS_MAGIC in jsapi.h, but jsval.h and jsvalue.h stuff about magic values.

Once this gets sorted, who should review this?

Comment 6

[David Mandelin [:dmandelin]]

(In reply to comment #5)
> Once this gets sorted, who should review this?

I think jorendorff or brendan would be good choices. If their reviewer queues are already too long, I could do it--I've worked on a good deal of function-related stuff.

Comment 7

[Brian Hackett [Laid off!]]

Attachment: patch without API changes

This patch rebases and removes all changes to jsapi.h --- the new JSFUN and JSCLASS flags for fast constructors are in jsfun.h.  trace-tests and jstests pass.  After talking with Luke, a good plan seems to be to get this patch in, introducing but not exposing fast constructors, then do a second patch for bug 581263 which puts the fast constructors in the API and removes slow natives/constructors.

Comment 8

[David Mandelin [:dmandelin]]

Comment on attachment 462413 [details] [diff] [review]
patch without API changes

Looks good. 2 small points:

- There is a 'TODO' remaining regarding the usage of flags/holes. This should be removed and filed as a followup bug instead. It seems OK to leave a brief note about the future plan in a comment, possibly even just "This is going to change someday, see bug XXXXXX."

- isMagic should be called with the form that takes an argument, just as an assertion check that we are getting the ctor-call-indicating hole rather than some other random kind.

Comment 9

[Brian Hackett [Laid off!]]

Attachment: patch fixing comments

Fix issues from comment 8.

Comment 10

[Brian Hackett [Laid off!]]

Attachment: MICs for native new

Use a MIC for native NEW.  There are only about 50k such calls in SS, so this saves ~1ms, but the symmetry with CALL is nice.

Comment 11

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 462623 [details] [diff] [review]
MICs for native new


>+        ncc.masm.storePayload(Imm32(JS_FAST_CONSTRUCTOR), Address(temp, sizeof(Value)));
>+        ncc.masm.storeTypeTag(ImmType(JSVAL_TYPE_MAGIC), Address(temp, sizeof(Value)));

nit: better if we can use masm.storeValue() here, which Sean pointed out is faster on x64.

Comment 12

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/f84b470314a8