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

Leverage micromatch to speed up glob matching #911

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mastrzyz
Copy link

micromatch is faster than minimatch in performing matching.

Which can be seen with auditable benchmarks here

I wanted to be 100% sure this would help this project and created two benchmark files of my own here trying to replicate some of the usecases of this project.

The results are :

marcinstrzyz@Marcins-MacBook-Pro-2 lib % node benchmarkLarge.js
Fastest is micromatch
┌─────────┬──────────────┬────────────────┬───────────────────────┬────────────────────────────┬──────────────────────────┐
│ (index) │   Function   │ Mean Time (ms) │ Operations per Second │ Standard Error of the Mean │ Relative Margin of Error │
├─────────┼──────────────┼────────────────┼───────────────────────┼────────────────────────────┼──────────────────────────┤
│    0    │ 'micromatch' │    '23.75'     │        '42.10'        │           '0.00'           │          '4.11'          │
│    1    │ 'minimatch'  │    '71.48'     │        '13.99'        │           '0.00'           │          '9.11'          │
└─────────┴──────────────┴────────────────┴───────────────────────┴────────────────────────────┴──────────────────────────┘
marcinstrzyz@Marcins-MacBook-Pro-2 lib % node benchmark.js     
Fastest is micromatch
┌─────────┬──────────────┬────────────────┬───────────────────────┬────────────────────────────┬──────────────────────────┐
│ (index) │   Function   │ Mean Time (ms) │ Operations per Second │ Standard Error of the Mean │ Relative Margin of Error │
├─────────┼──────────────┼────────────────┼───────────────────────┼────────────────────────────┼──────────────────────────┤
│    0    │ 'micromatch' │     '0.03'     │      '37700.76'       │           '0.00'           │          '3.83'          │
│    1    │ 'minimatch'  │     '0.06'     │      '15809.91'       │           '0.00'           │          '6.07'          │
└─────────┴──────────────┴────────────────┴───────────────────────┴────────────────────────────┴──────────────────────────┘

Although the perceived speedup may be only seen in very large cases, we do see it is 50% improvement overall.

The current UT's work fine, happy to extend them if we have some worries about known edge cases or we just want more security.

@mastrzyz
Copy link
Author

@kenotron thoughts on this? wondering if Lage could benefit it further downstream as well.

@mastrzyz
Copy link
Author

@ecraig12345 ?

@ecraig12345
Copy link
Member

My big concern with making this change in a non-major version is the difference in backslash handling. A lot of our internal teams use Windows, so it wouldn't be surprising if somebody is using backslashes as path separators in patterns, and it's hard to accurately detect and fix that.

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.

None yet

2 participants