1249235 - Store RegExp flags into single slot.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Tooru Fujisawa [:arai]]

Currently RegExp flags are stored into multiple slots, but they are stored with BooleanValue, and no other types would be used, as they're readonly (am I correct?).  so we could use single slot with Int32Value, with raw RegExpFlags value.  it reduces the RegExpObject's size and possibly improves performance.

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: Part 1: Store RegExp flags into single slot.

so, there was no need to use separated slots after bug 1120169.

AllocKind of RegExpObject is reduced from OBJECT8 to OBJECT4.

octane/regexp score improves about 2%:
  before: 4089
  after:  4176

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: Part 2: Optimize RegExp.prototype.flags to load flags at once.

Changing RegExp.prototype.flags to call all getters before other operations reduces the loadfixedslot instruction.

without this patch, it needs 2 loadfixedslot, before global and unicode.

Comment 3

[Hannes Verschore [:h4writer]]

Comment on attachment 8720770 [details] [diff] [review]
Part 1: Store RegExp flags into single slot.

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

Nice!

Comment 4

[Hannes Verschore [:h4writer]]

Comment on attachment 8720776 [details] [diff] [review]
Part 2: Optimize RegExp.prototype.flags to load flags at once.

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

I find this odd. In my knowledge the compiler should be able to transform this into one Load.
Looks good. Needinfo'ing me to investigate that.

Comment 5

[Tooru Fujisawa [:arai]]

To be clear, I used m-i 410ef34da2e7 revision and applied Part 1, and executed following code with `IONFLAGS=logs js --ion-offthread-compile=off test.js` with 64bit build on osx.

configure flags: --enable-debug --disable-optimize --enable-warnings-as-errors --enable-nspr-build

code:

let n = 0;
let re = /a/;
function f(R) {
    return R.flags;
}
for (let i = 0; i < 10000; i ++) {
    n += f(re).length;
}


I get func00, func01, and func02.  func00 and func01 are graph for RegExp.prototype.flags, and func02 is the graph that includes for loop and RegExp.prototype.flags.

in func00 and func01, loadfixedslot is executed twice, and in func02 it's executed only once, per single RegExp.prototype.flags call.

maybe func00 and func01 are the graph for not yet fully optimizated cases?

Comment 6

[Tooru Fujisawa [:arai]]

Attachment: func00-pass26-Allocate Registers [Backtracking]-lir.gv.png

Comment 7

[Tooru Fujisawa [:arai]]

Attachment: func01-pass26-Allocate Registers [Backtracking]-lir.gv.png

Comment 8

[Tooru Fujisawa [:arai]]

Attachment: func02-pass26-Allocate Registers [Backtracking]-lir.gv.png

Comment 9

[Tooru Fujisawa [:arai]]

oh, I was wrong, in func02 loadfixedslot is executed twice *before* the loop.

Comment 10

[Tooru Fujisawa [:arai]]

maybe, I've read the graph in totally wrong way.

even with Part 2, there are also 2 loadfixedslot's in func00, func01, and func02, with the code in comment #5.
so what the Part 2 does wasn't what I intended, sorry.

Comment 11

[Tooru Fujisawa [:arai]]

Attachment: part2-func00-pass26-Allocate Registers [Backtracking]-lir.gv.png

Comment 12

[Tooru Fujisawa [:arai]]

Attachment: part2-func01-pass26-Allocate Registers [Backtracking]-lir.gv.png

Comment 13

[Tooru Fujisawa [:arai]]

Attachment: part2-func02-pass26-Allocate Registers [Backtracking]-lir.gv.png

Comment 14

[Tooru Fujisawa [:arai]]

Comment on attachment 8720776 [details] [diff] [review]
Part 2: Optimize RegExp.prototype.flags to load flags at once.

as discussed in IRC, marking Part 2 obsolete, as it does not reduce the loadfixedslot.
will land Part 1 only after some more investigation.

Comment 15

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d1a4b82b556a5491cd824e70f781a55f7025269d
Bug 1249235 - Store RegExp flags into single slot. r=h4writer

Comment 16

[Guilherme Lima]

Apparently this patch and the others you landed with it, were a win on dromaeo-sunspider-string-validate-input (13% improvement) and made dromaeo-v8-richards stable.

Comment 17

[Phil Ringnalda (:philor)]

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

Comment 18

[Hannes Verschore [:h4writer]]

My needinfo is not needed anymore. Arai++ already fixed it.