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

Improve performance of SearchSource.findMatchingTests by 15% #8184

Conversation

scotthovestadt
Copy link
Contributor

Summary

This PR is a 15% performance improvement for SearchSource.findMatchingTests. I benchmarked a test data set of 300k files at 975ms~ with the current code. The same search pattern now executes in 830ms~.

At Facebook, this means that for the common use case of triggering jest MyFileName, Jest will launch >100ms faster. Sometimes you have to take the small wins. :)

I optimized this by looking for inefficiencies in code that is called for every file. In this case:

  • One of the inner functions triggered for each file called Object.keys for the stats object every time. Not efficient.
  • Dummy functions were being created that always returned true. It still costs CPU time to call a function, even an extremely fast one. Better to never call it.

I refactored away the issues by adapting the data structures to better fit the way they are being used. Since our stat filters are optional and we want to iterate them, they make more sense as an array. I maintained full type safety.

Test plan

  • All existing tests path, which already cover this functionality.
  • Careful benchmarking of the performance.

@codecov-io
Copy link

codecov-io commented Mar 21, 2019

Codecov Report

Merging #8184 into master will decrease coverage by <.01%.
The diff coverage is 76.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8184      +/-   ##
==========================================
- Coverage   62.32%   62.32%   -0.01%     
==========================================
  Files         265      265              
  Lines       10469    10465       -4     
  Branches     2545     2541       -4     
==========================================
- Hits         6525     6522       -3     
  Misses       3361     3361              
+ Partials      583      582       -1
Impacted Files Coverage Δ
packages/jest-core/src/getNoTestFoundVerbose.ts 0% <0%> (ø) ⬆️
packages/jest-core/src/SearchSource.ts 58.42% <85.18%> (-0.72%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f809c79...462d123. Read the comment docs.

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

nice! I thought the code looked weird when I migrated this to TS, makes sense to use an array instead here

(changelog)

Copy link
Collaborator

@thymikee thymikee left a comment

Choose a reason for hiding this comment

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

Sweet! Left some minor comments. Missing changelog

packages/jest-core/src/SearchSource.ts Outdated Show resolved Hide resolved
packages/jest-core/src/SearchSource.ts Outdated Show resolved Hide resolved
packages/jest-core/src/SearchSource.ts Show resolved Hide resolved
packages/jest-core/src/SearchSource.ts Show resolved Hide resolved
Copy link
Member

@cpojer cpojer left a comment

Choose a reason for hiding this comment

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

Nice!

Copy link
Collaborator

@thymikee thymikee left a comment

Choose a reason for hiding this comment

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

Thanks!

@github-actions
Copy link

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.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants