Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: hoist regex patterns #18438

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

GalacticHypernova
Copy link

@GalacticHypernova GalacticHypernova commented Oct 23, 2024

Description

Hoisting regex patterns from loops/hot code paths provides both better performance and better memory efficiency, both of which are especially invaluable for resource-constrained environments.

By reusing the hoisted pattern, the engine doesn't need to waste time on recompiling the pattern, thus performing better, and doesn't need to waste resources on local object creation and GC cycles, thus being more efficient.

The benchmark and memory profiling (of the benchmark) is taken from nuxt/nuxt#29620
Each part was ran separately to prevent any sort of inline cache from altering the results

const version = "1.0.0-1.0a"
console.time("inline")
for (let i=0;i<100000;i++){
  version.replace(/-\d+\.[0-9a-f]+/, '')
}
console.timeEnd("inline")

const SEMANTIC_VERSION_RE = /-\d+\.[0-9a-f]+/
console.time("external")
for (let i=0;i<100000;i++){
  version.replace(SEMANTIC_VERSION_RE, '')
}
console.timeEnd("external")

Results:

Browser:

image
image

Node:

image
image

It also shows a 50% decrease in memory usage in this simple benchmark alone, and has been tested using a memory profiler:

image
image

@GalacticHypernova
Copy link
Author

GalacticHypernova commented Oct 23, 2024

@patak-dev Hey! Apologies for the mention, but I wanted to ask which areas you'd like me to leave as is? I see some possible changes in create-vite, rollup.config.ts, and other, "normal" files in packages, and wanted to make sure what is off limits, if anything.

patak-dev
patak-dev previously approved these changes Oct 23, 2024
Copy link
Member

@patak-dev patak-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one looks good to me, thanks! If you see other similar changes, you can send other PRs to review them separatedly.

@GalacticHypernova
Copy link
Author

Can I combine them all in here so it could be merged at once? I found many places that can benefit from it, just wanted to know if any parts are not to be altered before I accidentally alter them

@patak-dev
Copy link
Member

Ya, that is ok too. Thanks!

@GalacticHypernova GalacticHypernova marked this pull request as ready for review October 26, 2024 14:21
@jaskp
Copy link

jaskp commented Nov 17, 2024

Nice 👍 I'm curious about how much it affects large builds. Peak memory usage reduction would be awesome

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