622557 - Removal of indexbase/resetbase

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, enhancement)

Description

[Igor Bukanov]

We should consider to remove INDEXBASE and RESETBASE prefix/suffix bytecodes and use 3-byte literal indexes in the bytecode to support over 64K of literals per script. With method and trace jits a potential performance loss in the interpreter should not be visisble while simpler bytecode handling would remove a source of critical bytes.

Comment 1

[Igor Bukanov]

About 2.0 nomination: although this bug is just an enhancement, implementing this would make bytecode handling simpler and remove poteny6ially rather bad bugs.

Comment 2

[Igor Bukanov]

Attachment: v1

The patch passes tests but I need to do some performance tests before asking for a review.

Comment 3

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

What about using 32-bit indexes?

Comment 4

[Igor Bukanov]

(In reply to comment #3)
> What about using 32-bit indexes?

I would like first try 3-byte indexes to clearly factor that out behind few inlines. Currently the assumption about 2-byte indexes is hardcoded in rather a few places.

After that we may consider alternatives like embedding atoms directly into the bytecode etc.

Comment 5

[Brendan Eich [:brendan]]

There's no point in four-byte indexes, due to

/*
 * Offset fields follow certain notes and are frequency-encoded: an offset in
 * [0,0x7f] consumes one byte, an offset in [0x80,0x7fffff] takes three, and
 * the high bit of the first byte is set.
 */
#define SN_3BYTE_OFFSET_FLAG    0x80
#define SN_3BYTE_OFFSET_MASK    0x7f

combined with

        index = js_NewSrcNote2(cx, cg, noteType, (ptrdiff_t) ALE_INDEX(label));
    return js_NewSrcNote2(cx, cg, SRC_FUNCDEF, (ptrdiff_t)index) >= 0 &&

at least. 23 bits is all you get.

We might even prefer frequency-encoding the atom index: if the high bit in the first byte is set, it's a three byte index, else a one-byte index. This makes for a branch in the interpreter that won't predict too well in general, but might be ok for lots of interpreter evaluations of a given script (where the immediate is always one byte or always three bytes).

/be

Comment 6

[Igor Bukanov]

Attachment: v2

The patch passes try server and shell tests.

With the patch I do not see differences with V8 and SS scores when run in the interpreter mode or with -m/-j flags. For better expose of index size increase and an extra shift/add per literal index that the interpreter now uses I created the following test:

var obj = { a:1, b:2, c: 3, d:4, f: function(N) {
        var s = 0;
        for (; N != 0; --N) {
            s += this.a + this.b + this.c + this.d;
        }
        return s;
    }};

obj.f(1e4);

var t = Date.now();
obj.f(1e7);
print(Date.now() - t);

In the test the density of literal index bytecodes in the loop is about 50%. With or without patch its gives the same timing with pure interpreter mode - about 1.51 seconds +- noise.

Comment 7

[LegNeato]

As per today's meeting, beta 9 will be a time-based release. Marking these all betaN+. Please move it back to beta9+ if you believe it MUST be in the next beta (ie: trunk is in an unshippable state without this)

Comment 8

[LegNeato]

Fixing fields my automated script accidentally blanked. Apologies for the bugspam

Comment 9

[Brendan Eich [:brendan]]

Comment on attachment 501085 [details] [diff] [review]
v2

I'm not convinced we need this for Firefox 4 / Mozilla 2. I could be wrong. If we don't need it, though, I'd like to see more experiments run on variable-length immediate coding, word-coding, other ideas, for lots of immediates.

/be

Comment 10

[Nicholas Nethercote [inactive]]

(In reply to comment #9)
> 
> I'm not convinced we need this for Firefox 4 / Mozilla 2. 

I agree -- this shouldn't be a softblocker.  It's even marked as an "enhancement".

Comment 11

[Brendan Eich [:brendan]]

Comment on attachment 501085 [details] [diff] [review]
v2

Aren't we going a different direction, to cut down on bytecode population: some variable-length coding of indexes (and jump offsets)?

/be