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

Optimise default glue paths in settings #232

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

kieran-ryan
Copy link
Sponsor Member

@kieran-ryan kieran-ryan commented May 13, 2024

🤔 What's changed?

  • Optimised default glue paths to merge repeated patterns to improve performance

⚡️ What's your motivation?

  • As the extension performs a complete reindex on code changes for specified glob paths, it is important that these paths are optimal for best performance
  • Code volume is reduced considerably, making it clearer which file extensions are expected under common paths

🏷️ What kind of change is this?

  • 🏦 Refactoring/debt/DX (improvement to code design, tooling, etc. without changing behaviour)

📋 Checklist:

  • I agree to respect and uphold the Cucumber Community Code of Conduct
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.
  • Complete benchmark of the changes

@kieran-ryan kieran-ryan added the 🏦 debt Tech debt label May 13, 2024
@kieran-ryan kieran-ryan self-assigned this May 13, 2024
@kieran-ryan
Copy link
Sponsor Member Author

kieran-ryan commented May 14, 2024

Benchmarked the fast-glob globbing library used by the Language Server through the node-glob benchmark.

Negligible difference for synchronous operations and interestingly the existing pattern appears to be around 17% faster (0.069s -> 0.081s); which is fairly insubstantial and may be a favourable tradeoff for a simplified default configuration. Though present performance constraints (#152) must also be considered.

@kieran-ryan ➜ /workspaces/node-glob (main) $ npm run bench

> glob@10.3.15 prebench
> npm run prepare


> glob@10.3.15 prepare
> tshy


> glob@10.3.15 bench
> bash benchmark.sh


--- pattern: 'features/**/*{.js,.jsx,.php,.py,.rb,.rs,.ts,.tsx,_test.go,.java}' ---
[ 'features/**/*{.js,.jsx,.php,.py,.rb,.rs,.ts,.tsx,_test.go,.java}' ]
~~ sync ~~
node fast-glob sync             0m0.089s  0
node globby sync                0m0.090s  0
node current globSync mjs       0m0.068s  0
node current glob syncStream    0m0.067s  0
~~ async ~~
node fast-glob async            0m0.081s  0
node globby async               0m0.092s  0
node current glob async mjs     0m0.068s  0
node current glob stream        0m0.067s  0

--- pattern: 'features/**/*.js features/**/*.jsx features/**/*.php features/**/*.py features/**/*.rb features/**/*.rs features/**/*.ts features/**/*.tsx features/**/*_test.go features/**/*.java' ---
[ 'features/**/*.js', 'features/**/*.jsx', 'features/**/*.php', 'features/**/*.py', 'features/**/*.rb', 'features/**/*.rs', 'features/**/*.ts', 'features/**/*.tsx', 'features/**/*_test.go', 'features/**/*.java' ]
~~ sync ~~
node fast-glob sync             0m0.090s  0
node globby sync                0m0.090s  0
node current globSync mjs       0m0.063s  0
node current glob syncStream    0m0.081s  0
~~ async ~~
node fast-glob async            0m0.069s 0
node globby async               0m0.093s  0
node current glob async mjs     0m0.067s  0
node current glob stream        0m0.117s  0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏦 debt Tech debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant