Allow nursery-allocating most RegExp objects

RESOLVED FIXED in Firefox 55

Status

()

Core
JavaScript Engine
RESOLVED FIXED
3 months ago
3 months ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

(Blocks: 1 bug)

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [qf])

Attachments

(2 attachments)

(Assignee)

Description

3 months ago
Created attachment 8872315 [details] [diff] [review]
Part 1 - Nursery-allocate in CloneRegExpObject

See bug 1368419.

Nursery-allocating in CloneRegExpObject wins a few hundred points on Octane-regexp and when I run just the six-speed regex-u tests:

before:

regex-u-es5: 197
regex-u-es6: 254

after:

regex-u-es5: 108
regex-u-es6: 158
Attachment #8872315 - Flags: review?(jcoppeard)
(Assignee)

Comment 1

3 months ago
Actually, js::RegExpAlloc can also nursery allocate in a lot of cases, for instance when it's called by the RegExpCreate intrinsic. I'll post another patch for that.
(Assignee)

Comment 2

3 months ago
Created attachment 8872317 [details] [diff] [review]
Part 2 - Pass NewObjectKind to RegExpAlloc

Passes NewObjectKind to RegExpAlloc so the parser/XDR/etc can allocate tenured but most other callers can nursery allocate.

This improves the following micro-benchmark from 375 to 283 ms:
--
function f() {
    var re;
    var t = new Date;
    for (var i = 0; i < 1000000; i++)
	re = new RegExp("foo");
    print(new Date - t);
    return re;
}
f();
--

And this one from 285 to 183 ms (although we can do much better here I think).
--
function f() {
    var res;
    var t = new Date;
    for (var i = 0; i < 1000000; i++)
	res = "foo12345".match(i % 128);
    print(new Date - t);
    return res;
}
f();
--
Attachment #8872317 - Flags: review?(jcoppeard)
(Assignee)

Updated

3 months ago
Attachment #8872315 - Attachment description: Patch → Part 1 - Nursery-allocate in CloneRegExpObject
(Assignee)

Comment 3

3 months ago
Good QF bug, makes things faster and helps avoid full GCs.
Blocks: 1351769
Summary: Allow nursery-allocating cloned RegExp objects → Allow nursery-allocating most RegExp objects
Whiteboard: [qf]

Comment 4

3 months ago
Comment on attachment 8872315 [details] [diff] [review]
Part 1 - Nursery-allocate in CloneRegExpObject

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

Nice!
Attachment #8872315 - Flags: review?(jcoppeard) → review+

Updated

3 months ago
Attachment #8872317 - Flags: review?(jcoppeard) → review+

Comment 5

3 months ago
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5be34c2d1de5
part 1 - Allow nursery allocation for RegExps allocated by CloneRegExpObject. r=jonco
https://hg.mozilla.org/integration/mozilla-inbound/rev/5f12fd37c3bd
part 2 - Pass NewObjectKind to RegExpAlloc so most callers can use nursery allocation. r=jonco

Comment 6

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5be34c2d1de5
https://hg.mozilla.org/mozilla-central/rev/5f12fd37c3bd
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.