Skip to content

perf(compression): use regex exec instead of match#210

Merged
lukeed merged 1 commit intolukeed:nextfrom
bluwy:use-regex-exec
Jul 27, 2024
Merged

perf(compression): use regex exec instead of match#210
lukeed merged 1 commit intolukeed:nextfrom
bluwy:use-regex-exec

Conversation

@bluwy
Copy link
Copy Markdown

@bluwy bluwy commented Jul 26, 2024

I was running eslint-plugin-regexp in polka and found we can refactor the str.match(re) to re.exec(str) for perf.

/Users/bjorn/Work/oss/vite/node_modules/.pnpm/@polka+compression@1.0.0-next.25/node_modules/@polka/compression/build.js
  30:32  error  Use the `RegExp#exec()` method instead  regexp/prefer-regexp-exec
  30:68  error  Use the `RegExp#exec()` method instead  regexp/prefer-regexp-exec

I made some tests and can confirm there's a sizable perf improvement with exec. (perf.link test)

@kurtextrem
Copy link
Copy Markdown

might make sense to hoist the regexps too?

@bluwy
Copy link
Copy Markdown
Author

bluwy commented Jul 27, 2024

The perf improvement from hoisting regex varies quite a bit in my experience and often doesn't yield noticable returns. I think engines are able to optimize them well enough these days. Maybe those with g flags could still be useful to hoist as they're more stateful.

@lukeed lukeed merged commit 822c80a into lukeed:next Jul 27, 2024
@lukeed
Copy link
Copy Markdown
Owner

lukeed commented Jul 27, 2024

released as @polka/compression@1.0.0-next.26, ty @bluwy

@bluwy bluwy deleted the use-regex-exec branch July 27, 2024 14:43
@kurtextrem
Copy link
Copy Markdown

@bluwy Last time we benched it for valibot, it was ~2ms faster to hoist (even non global regexps)

@bluwy
Copy link
Copy Markdown
Author

bluwy commented Jul 27, 2024

Yeah it'll depend but it's often a very small amount that I've come to prefer not bugging libraries to hoist them unless really needed 😅

@lukeed
Copy link
Copy Markdown
Owner

lukeed commented Jul 27, 2024

It generally has a significant impact in repeat synchronous usage (eg loops), so seeing it in valibot makes sense. In an async-like callback (like here) it won’t matter much, especially with simple patterns

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants