Closed
Bug 1368461
Opened 6 years ago
Closed 6 years ago
Allow nursery-allocating most RegExp objects
Categories
(Core :: JavaScript Engine, enhancement)
Core
JavaScript Engine
Tracking
()
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: jandem, Assigned: jandem)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
2.05 KB,
patch
|
jonco
:
review+
|
Details | Diff | Splinter Review |
13.93 KB,
patch
|
jonco
:
review+
|
Details | Diff | Splinter Review |
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•6 years 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•6 years ago
|
||
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•6 years ago
|
Attachment #8872315 -
Attachment description: Patch → Part 1 - Nursery-allocate in CloneRegExpObject
Assignee | ||
Comment 3•6 years 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•6 years 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•6 years ago
|
Attachment #8872317 -
Flags: review?(jcoppeard) → review+
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•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5be34c2d1de5 https://hg.mozilla.org/mozilla-central/rev/5f12fd37c3bd
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Updated•1 year ago
|
Performance Impact: --- → ?
Whiteboard: [qf]
You need to log in
before you can comment on or make changes to this bug.
Description
•