4.4x slower than Chrome on the single String.prototype.split call in Editor-CodeMirror
Categories
(Core :: JavaScript Engine, defect, P2)
Tracking
()
People
(Reporter: mstange, Unassigned)
References
(Blocks 2 open bugs)
Details
(Whiteboard: [sp3])
Of the overall measured time in the Editor-CodeMirror subtest, 15% is taken up by a single call to String.prototype.split
. It's splitting 29869 line string (the contents of this file) by the regular expression /\r\n?|\n/
.
Reducing our time to Chrome's would win us 11% on this subtest.
A considerable portion of our time is spent in GC.
Firefox: https://share.firefox.dev/3Y3KFB1 (3715 samples)
Chrome: https://share.firefox.dev/3K9pvvw (829 samples)
Updated•1 year ago
|
Comment 1•1 year ago
|
||
We're calling RegExpMatcher
from RegExpSplit
and that's pretty wasteful because it allocates a full match result object each time.
It's likely common for the regexp to have no capture groups. We could check for that and then potentially make use of my patches in bug 1839422. Ideally we wouldn't have a perf cliff if capture groups are used though. I'll take a look at those internal RegExpMatcher
calls.
Comment 2•1 year ago
|
||
V8 is generating an optimized path for regexp-split here: https://source.chromium.org/chromium/chromium/src/+/main:v8/src/builtins/builtins-regexp-gen.cc;l=1475;drc=2f1ca5534e77fb452c220ed8b1da32c359fbcd6e
Comment 3•11 months ago
|
||
I've added an optimization for regexp-split-without-capture-groups on top of my patches in bug 1839422.
Perf results show that Editor-CodeMirror/Long/Sync improves by 29%, Editor-CodeMirror/Long/total by 19% and Editor-CodeMirror/total by 10%.
We could maybe make this a little bit faster by inlining the new intrinsic in the JITs, I'm working on a patch for that too.
Although most of the calls are with a regexp without capture groups, Speedometer 3 unfortunately also has a bunch of calls for the following monster that does have capture groups:
('(?:(?:\\')?[^'])*'|"(?:(?:\\")?[^"])*"|\s+|\btrue\b|\bfalse\b|\bintersect\b|//|==|!=|>=|<=|&&|\|\||\bin\b|\.|\[|\]|\||\{|\}|:|,|\(|\)|\?|\+|-|\*|/|%|\^|>|<|!|\b[a-zA-Z_\$][a-zA-Z0-9_\$]*\b|(?:(?:[0-9]*\.[0-9]+)|[0-9]+))
Comment 4•11 months ago
|
||
(In reply to Jan de Mooij [:jandem] from comment #3)
Although most of the calls are with a regexp without capture groups, Speedometer 3 unfortunately also has a bunch of calls for the following monster that does have capture groups:
Oh nevermind, this isn't in Speedometer 3. I think it's chrome code in toolkit/components/utils/mozjexl.js
Comment 5•11 months ago
|
||
This should be a lot closer to Chrome now after bug 1839422 has landed. If you have a chance to confirm this when you're back that'd be great.
Fixing bug 1846297 might also improve this test a little.
Updated•11 months ago
|
Reporter | ||
Comment 6•11 months ago
|
||
(In reply to Jan de Mooij [:jandem] from comment #5)
This should be a lot closer to Chrome now after bug 1839422 has landed.
Indeed, it improved from 4.4x of Chrome to 1.5x of Chrome. Nice work!
Firefox: https://share.firefox.dev/3qHK1gi (1300 samples)
Chrome: https://share.firefox.dev/45r9l98 (867 samples)
Comment 7•11 months ago
|
||
(In reply to Markus Stange [:mstange] from comment #6)
Indeed, it improved from 4.4x of Chrome to 1.5x of Chrome. Nice work!
Firefox: https://share.firefox.dev/3qHK1gi (1300 samples)
Chrome: https://share.firefox.dev/45r9l98 (867 samples)
Thanks for the new profile. It looks a lot better than the one in comment 0 :) The main thing that stands out now is the nursery GC, probably because we allocate so many dependent strings for the result array.
Description
•