Skip to content

Updates spk's FileMatcher to lazily create its internal gitignore object#1290

Open
dcookspi wants to merge 3 commits intomainfrom
update-FileMatcher-to-lazy-init-gitignore-object
Open

Updates spk's FileMatcher to lazily create its internal gitignore object#1290
dcookspi wants to merge 3 commits intomainfrom
update-FileMatcher-to-lazy-init-gitignore-object

Conversation

@dcookspi
Copy link
Copy Markdown
Collaborator

@dcookspi dcookspi commented Nov 12, 2025

This updates spk's FileMatcher objects to lazily create its internal gitignore object on first call to the matches() method.

The FileMatcher and its gitignore field was accounting for about a quarter of the load/read time for a package build spec. The matching is only carried out when building a package, so setting it up lazily saves time in other parts of spk (e.g. a solve that reads 15k package build specs).

Todo:

  • Add check for empty rules before creating a gitignore
  • Include result to avoid repeated recreation failures
  • Profile vs ArcSwap

@dcookspi dcookspi requested review from jrray and rydrman November 12, 2025 19:40
@dcookspi dcookspi self-assigned this Nov 12, 2025
@dcookspi dcookspi added SPI AOI Area of interest for SPI enhancement New feature or request labels Nov 12, 2025
@codecov
Copy link
Copy Markdown

codecov bot commented Nov 12, 2025

Codecov Report

❌ Patch coverage is 63.63636% with 12 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ema/crates/foundation/src/spec_ops/file_matcher.rs 62.50% 12 Missing ⚠️

📢 Thoughts on this report? Let us know!

pub struct FileMatcher {
rules: Vec<String>,
gitignore: ignore::gitignore::Gitignore,
gitignore: Arc<Mutex<Option<ignore::gitignore::Gitignore>>>,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Profile first, but ArcSwap may be a cheaper way to manage this field. Instead of creating numerous Mutex instances. I'm more concerned about this significantly slowing down builds for packages with a lot of files and calling matches on each file.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

maybe a OnceCell to leverage get_or_try_init? Maybe that's been tried, but leaving the note anyway

@dcookspi dcookspi force-pushed the update-FileMatcher-to-lazy-init-gitignore-object branch from 1c26c2e to f01b410 Compare December 18, 2025 01:50
@dcookspi
Copy link
Copy Markdown
Collaborator Author

In a discussion today, we realised that this PR is about to be redundant because the type splitting between recipe and package (build spec) will mean the FileMatcher field won't be in the v1 packages any more.

… first call to matches() method.

Signed-off-by: David Gilligan-Cook <dcook@imageworks.com>
…hing lazy initialisation.

Signed-off-by: David Gilligan-Cook <dcook@imageworks.com>
…dly trying to initialise it when that will fail

Signed-off-by: David Gilligan-Cook <dcook@imageworks.com>
@dcookspi dcookspi force-pushed the update-FileMatcher-to-lazy-init-gitignore-object branch from f01b410 to 65c410e Compare March 17, 2026 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request SPI AOI Area of interest for SPI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants