Skip to content

Conversation

effigies
Copy link
Contributor

This PR has two pieces, neither of which produce any noticeable speedup on their own. The first creates a context.loaded Promise that indicates whether the context is fully loaded. The second creates a queue that can hold onto initializing contexts until we're ready for them.

For ds000001, I was getting about 9s per run. With a queue of 5, down to 4s, and a queue of 20 got us down to 2s.

@effigies effigies requested a review from rwblair October 14, 2025 19:07
@effigies effigies changed the title feat: Start loading contexts earlier, buffer contexts for better parallelism feat: Start loading contexts earlier, buffer contexts for better concurrency Oct 14, 2025
Copy link

codecov bot commented Oct 14, 2025

Codecov Report

❌ Patch coverage is 94.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.35%. Comparing base (e208822) to head (ac133ef).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
src/schema/walk.ts 81.25% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #282      +/-   ##
==========================================
+ Coverage   87.32%   87.35%   +0.03%     
==========================================
  Files          49       50       +1     
  Lines        3620     3662      +42     
  Branches      593      600       +7     
==========================================
+ Hits         3161     3199      +38     
- Misses        449      453       +4     
  Partials       10       10              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@effigies
Copy link
Contributor Author

Forgot to run tests... Just validated.

@effigies effigies marked this pull request as draft October 14, 2025 19:14
@effigies effigies force-pushed the feat/context-queue branch 2 times, most recently from 5faa36b to b352770 Compare October 14, 2025 23:00
@effigies effigies marked this pull request as ready for review October 14, 2025 23:02
@effigies
Copy link
Contributor Author

Okay, reworked this to perform new BIDSContext() and await context.loaded in walkFileTree or in a helper async function makeBIDSContext(). As long as nobody calls new BIDSContext() directly instead of through these, the resources should get released.

@rwblair rwblair merged commit acc89e7 into bids-standard:main Oct 15, 2025
29 checks passed
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.

2 participants