-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Performance: Cache regular expression instead of creating anew for every file in ScriptTransformer. #8235
Performance: Cache regular expression instead of creating anew for every file in ScriptTransformer. #8235
Conversation
…cript transformer.
Codecov Report
@@ Coverage Diff @@
## master #8235 +/- ##
==========================================
+ Coverage 62.34% 62.36% +0.01%
==========================================
Files 265 265
Lines 10559 10569 +10
Branches 2568 2570 +2
==========================================
+ Hits 6583 6591 +8
- Misses 3387 3388 +1
- Partials 589 590 +1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
if (new RegExp(this._config.transform[i][0]).test(filename)) { | ||
return this._config.transform[i][1]; | ||
const transformRegExp = this._cache.transformRegExp; | ||
if (!transformRegExp) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When can this be falsy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the array passed from config with transforms is 0-length.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, transform: {}
? Gotcha
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
calcTransformRegExp
returns undefined
if config.transform.length === 0
.
I try not to return empty arrays to loop through when the function should actually be telling you "nothing to do here". I personally find it makes the code more readable and sometimes make optimizations in the future possible.
if ( | ||
!config.transformIgnorePatterns || | ||
config.transformIgnorePatterns.length === 0 | ||
) { | ||
return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
This PR improves performance by caching the regular expression string ->
RegExp
conversion.Benchmarks on cached regular expression vs. creating new each time:
For large projects, Jest may process a LOT of files. The more workers in use, the higher chance that the same file is processed multiple times. Additionally, each time it's hit multiple regular expressions may be tested against.
The benchmark I used:
Test plan