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

Performance improvements #86

Merged
merged 7 commits into from
Mar 17, 2022
Merged

Performance improvements #86

merged 7 commits into from
Mar 17, 2022

Conversation

TwitchBronBron
Copy link
Member

@TwitchBronBron TwitchBronBron commented Mar 15, 2022

Fixes several performance issues related to globbing for files.

  • run all glob lookups in parallel instead of sequentially
  • negative globs no longer run a glob and then remove those entries from the overall result. Instead, the negative globs are used to filter previous matches. This is significantly faster.
  • switch to a faster glob library. We currently use node-glob. Based on my preliminary tests, fdir is the fastest. However, there's a bug in that library right now related to symlinks, so for now we will use fast-glob. (EDIT: fdir has a few other design differences that prevent us from switching. fast-glob is plenty fast).
    image

To benchmark this, I ran the following roku-deploy command against a folder with >64,000 files:

await rokuDeploy.getFilePaths([
    '**/*',
    '!**/node_modules/**/*'
], "path/to/folder");

Results:
baseline (before rewrite):
24.409 ops/sec, duration: 47s318ms

after rewrite (using node-glob):
916.974 ops/sec, duration: 1s626ms

after rewrite (using fast-glob):
3,147.138 ops/sec, duration: 0s367ms

@TwitchBronBron TwitchBronBron marked this pull request as draft March 15, 2022 15:27
@TwitchBronBron TwitchBronBron force-pushed the performance-improvements branch from 01b9379 to 94f0ca1 Compare March 15, 2022 18:35
@TwitchBronBron TwitchBronBron force-pushed the performance-improvements branch from 94f0ca1 to 3024166 Compare March 15, 2022 19:40
@TwitchBronBron TwitchBronBron marked this pull request as ready for review March 16, 2022 19:19
Copy link

@livecano livecano left a comment

Choose a reason for hiding this comment

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

Very nice!

@TwitchBronBron TwitchBronBron merged commit 26a762b into master Mar 17, 2022
@TwitchBronBron TwitchBronBron deleted the performance-improvements branch March 17, 2022 12:38
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.

4 participants