1642374 - Irregexp: Evaluate global caching optimization (RegExpGlobalExecRunner)

Status: NEW

(Core :: JavaScript Engine, enhancement, P2)

Description

[Iain Ireland [:iain]]

Attachment: irregexp-global-cache.patch

Irregexp supports a mode of execution for compiled global regexps where the match results buffer is larger and multiple matches are found in a single execution, then cached for later. In our original import of irregexp, bhackett included support for this mode in the macro-assembler, but it was never hooked up to anything.

I don't know how much of a performance improvement to expect from this, but I had already written most of the code before I realized that we weren't using it. I'm attaching a patch that adds the masm support, guarded behind ENABLE_GLOBAL_CACHE.

Presumably the first step would be to try disabling this optimization in V8 to measure how much benefit they get from it.

Comment 1

[Iain Ireland [:iain]]

Apparently V8 is seeing enough of a win from this to enable calling it from their code stub assembler: https://chromium-review.googlesource.com/c/v8/v8/+/5954350.

Comment 2

[Iain Ireland [:iain]]

It looks like V8 is also working to expand their use of this optimization for the purposes of jetstream performance: https://issues.chromium.org/issues/372285209. Using it for things like split seems especially promising, given that split shows up in sp3 profiles.

It's possible that some of the gap between us and V8 on regexp builtins is attributable to this optimization.

js-perf-next: Disable this optimization in V8 to see how much of a performance win they're actually getting on Speedometer. If it's significant, we should strongly consider implementing this. The irregexp side is relatively straightforward / already mostly done in the attached patch, but there would be significant work in the self-hosted builtins / regexp stubs.

Comment 3

[Dan Minor [:dminor]]

I spent some time investigating RegExpResultsCache, which is controlled by the --no-regexp-results-cache flag. This does not appear to have much of an effect on overall Speedometer or JetStream results, at least in testing on my development system. Whatever effect it does have, is smaller than the variance I see between runs. As it turned out, however, this was not the correct thing to be investigating :/

This bug is actually about the RegExpGlobalExecRunner, which was formally called RegExpGlobalCache. There's no flag to enable or disable this feature, so I did some experimentation with local builds of v8. In particular, I built a version of v8 from commit dbefb3c8aeb4, the first in the series on https://issues.chromium.org/issues/372285209, and from commit 0362abd3c829b, the last in the series on that issue.

From dbefb3c8aeb4:

Total Score: 206.092

Running regexp:
Startup: 312.500
Worst Case: 363.636
Average: 438.467
Score: 367.975
Wall time: 0:01.408

From 0362abd3c829:

Total Score: 217.519

Running regexp:
Startup: 454.545
Worst Case: 384.615
Average: 503.384
Score: 444.803
Wall time: 0:01.221

Which shows a significant improvement over that series of patches, although of course, it's possible other improvements landed in the month that elapsed between the first commit on that issue and the last.

I think these results are promising enough to experiment with implementing this in SpiderMonkey.

Comment 4

[Dan Minor [:dminor]]

Attachment: Bug 1642374 - Return number of matches from regexp functions; r=iain!

Comment 5

[Dan Minor [:dminor]]

Attachment: Bug 1642374 - Use Span to update MatchPairs in RegExpStatics; r=iain!

When we start returning multiple results, we'll only want to store the
MatchPairs corresponding to the last result in RegExpStatics.

Comment 6

[Dan Minor [:dminor]]

Attachment: Bug 1642374 - Add masm support for irregexp global cache; r=iain!

Comment 7

[Dan Minor [:dminor]]

Attachment: WIP: Bug 1642374 - Add RegExpGlobalSearcher and use it for match

Comment 8

[Dan Minor [:dminor]]

Attachment: WIP: Bug 1642374 - Enable global cache for string replace (WIP)

Comment 9

[Dan Minor [:dminor]]

Attachment: WIP: Bug 1642374 - Make RegExpGlobalSearcher inlineable (WIP)