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.

Sign up for GitHub

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

Jump to bottom

module: cache regular expressions #3869

Closed
wants to merge 1 commit into from
Closed

module: cache regular expressions #3869

wants to merge 1 commit into from

Conversation

evanlucas
Copy link
Contributor

None of these regular expressions will change, so there is no need to
generate them every time in hot code paths.

Provides a small performance improvement in module loading. (5-10%)

@evanlucas
module: cache regular expressions
a4b8215 
None of these regular expressions will change, so there is no need to
generate them every time in hot code paths.

Provides a small performance improvement in module loading. (5-10%)
@evanlucas evanlucas added the module Issues and PRs related to the module subsystem. label Nov 17, 2015
@cjihrig
Copy link
Contributor

cjihrig commented Nov 17, 2015

LGTM

@evanlucas
Copy link
Contributor Author

CI: https://ci.nodejs.org/job/node-test-pull-request/755/

@ronkorving
Copy link
Contributor

Wow, that gives a measurable improvement? I'm kinda surprised V8 doesn't just keep this in cache (it has a compiled regex cache, right?).

@jasnell
Copy link
Member

jasnell commented Nov 17, 2015

Nice!
LGTM

@jasnell
Copy link
Member

jasnell commented Nov 17, 2015

out of curiosity, @evanlucas did you have benchmark numbers that show the improvement?

evanlucas added a commit that referenced this pull request Nov 17, 2015
@evanlucas @jasnell
module: cache regular expressions
4fed62c 
None of these regular expressions will change, so there is no need to
generate them every time in hot code paths.

Provides a small performance improvement in module loading. (5-10%)

PR-URL: #3869
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@jasnell
Copy link
Member

jasnell commented Nov 17, 2015

Landed in 4fed62c

@jasnell jasnell closed this Nov 17, 2015
@evanlucas
Copy link
Contributor Author

Sure, here are a couple runs

$ ./node benchmark/compare.js ./node ./node_before -- misc module-loader
running ./node
misc/module-loader.js
running ./node_before
misc/module-loader.js

misc/module-loader.js thousands=50: ./node: 5.0003 ./node_before: 4.7087 . 6.19%

$ ./node benchmark/compare.js ./node ./node_before -- misc module-loader
running ./node
misc/module-loader.js
running ./node_before
misc/module-loader.js

misc/module-loader.js thousands=50: ./node: 5.0322 ./node_before: 4.7896 . 5.06%

@ronkorving
Copy link
Contributor

Higher is better, right? :)

evanlucas added a commit that referenced this pull request Nov 17, 2015
@evanlucas @Fishrock123
module: cache regular expressions
5c54fa0 
None of these regular expressions will change, so there is no need to
generate them every time in hot code paths.

Provides a small performance improvement in module loading. (5-10%)

PR-URL: #3869
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
evanlucas added a commit that referenced this pull request Dec 1, 2015
@evanlucas
module: cache regular expressions
746e279 
None of these regular expressions will change, so there is no need to
generate them every time in hot code paths.

Provides a small performance improvement in module loading. (5-10%)

PR-URL: #3869
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
evanlucas added a commit that referenced this pull request Dec 4, 2015
@evanlucas @rvagg
module: cache regular expressions
8a264ae 
None of these regular expressions will change, so there is no need to
generate them every time in hot code paths.

Provides a small performance improvement in module loading. (5-10%)

PR-URL: #3869
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@jasnell jasnell mentioned this pull request Dec 17, 2015
Closed
evanlucas added a commit that referenced this pull request Dec 17, 2015
@evanlucas @jasnell
module: cache regular expressions
971762a 
None of these regular expressions will change, so there is no need to
generate them every time in hot code paths.

Provides a small performance improvement in module loading. (5-10%)

PR-URL: #3869
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
evanlucas added a commit that referenced this pull request Dec 23, 2015
@evanlucas @jasnell
module: cache regular expressions
9b713f1 
None of these regular expressions will change, so there is no need to
generate them every time in hot code paths.

Provides a small performance improvement in module loading. (5-10%)

PR-URL: #3869
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
module Issues and PRs related to the module subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@evanlucas @cjihrig @ronkorving @jasnell @MylesBorins