perf: ignore instead of wax #4578

tdejager · 2025-09-12T12:37:46Z

Experiment with going back to ignore, seems quite promising. Got it to 1ms for most minimal folder structure for glob comparison in debug as opposed to 1s worst-case with wax on my machine. The exact problem that it was parsing too many directories now seems to happen with wax as opposed to ignore, I have an external test that I verified with @ruben-arts where I saw that happening. I'll link that in a bit.

Update 18-09:

Why?

We had a lot of slowness in glob traversal when testing out pixi build, seems that wax in the released version was iterating into the .pixi folder when it should not need to. We've switched to ignore, which seemed to have worse performance charateristics in the past. But this has changed.

Description

This is now in a state that it can be merged, this completely moves away from wax for now, as it seems in the use-cases that we are interested in I cannot get it to perform faster. I've also made a very naive implementation that does a BFS with fastglob (together with @remimimimimi) in: tdejager/wax-vs-ignore and it seems to be to always be faster or equally fast.

What changed?

High-level overview of the things I did:

Completely replace GlobSet with ignore implementation.
Add GlobWalkRoot that determines where to start walking and allows rebasing of globs. This was needed because we did multiple walks with wax depending on the glob which could contain seperators like ... This means you need to start walking from a location that is offset from the searchpath. Because I want to ideally do everything in one walk. I inverted this problem that we rebase/reset the globs that do not have any .. relative seperators. This allows us to use all globs in a single traversal. You can read the module glob_walk_root.rs for an overview, and I also added a lot of tests to verify the correctness for this behavior.
I essentially split up the actual walking in a walk.rs file. We use a feature of the ignore library to be able to walk over directories in a parallel. We use the visit method with a custom visitor that collects all values when the visitors are dropped. This makes sure we only need to synchronize in the end, I think the mutex here is not needed per se because the joins are serial. However, this makes sure it does not break in the future when things do change. Using this approached saves like 10ms for a big project like opencv on my machine.
I've kept the timings in debug so we can check it in the coming releases if reports come in that its slow.

How this was tested.

This is it running on the opencv repo by @ruben-arts:

The runtimes vary a bit though, but its a far cry from the 400ms it could sometimes take in the best case.

I've also added a significant number of unit tests.

Review

Assigning to @nichmor and @remimimimimi. @nichmor seeings as you made the semantic glob adjustments in wax, would be great to closely check glob_walk_root.rs.

ruben-arts · 2025-09-12T13:20:46Z

Now on opencv, it takes ~40ms: https://share.firefox.dev/4n3ZTSu
Insane improvement over the latest release with 800ms: https://share.firefox.dev/4nlupXI

I do think this is still too much for something that runs on every pixi run, as in a big mono repo, it could easily grow again.

tdejager · 2025-09-12T13:49:26Z

And thats in release right @ruben-arts?

ruben-arts · 2025-09-12T13:52:38Z

Yes that was in release mode.

Hofer-Julian · 2025-09-12T14:08:12Z

On my linux machine I get the following:

ignore: ~20ms: https://share.firefox.dev/4nqyvh0
wax: ~250ms: https://share.firefox.dev/42lgR6m

tdejager · 2025-09-13T11:49:43Z

I made a small comparison repository, to see if I got things right. Maybe @baszalmstra can check if he sees any mistakes. https://github.com/tdejager/wax-vs-ignore

tdejager · 2025-09-18T11:59:53Z

crates/pixi_glob/src/glob_set/walk_root.rs

@@ -0,0 +1,437 @@
+//! Plan the effective glob walk root for a set of patterns that may contain relative components.


@nichmor please check this :)

tdejager · 2025-09-18T12:00:23Z

crates/pixi_glob/src/glob_set/walk.rs

+    fn drop(&mut self) {
+        let mut sink = self.sink.lock();
+        sink.get_or_insert_with(Vec::new).append(&mut self.local);
+    }


This is where we append all results from the walk

tdejager · 2025-09-18T12:00:38Z

crates/pixi_glob/src/glob_set/walk.rs

+        .git_ignore(true)
+        .git_exclude(true)
+        .hidden(true)


I've set some defaults here.

tdejager · 2025-09-18T12:00:58Z

Cargo.toml

 uv-pypi-types = { git = "https://github.com/astral-sh/uv", tag = "0.8.5" }
 uv-requirements-txt = { git = "https://github.com/astral-sh/uv", tag = "0.8.5" }

-wax = "0.6.0"


Gone with ye, for now!

remimimimimi · 2025-09-18T13:02:02Z

Could you fix the tests @tdejager? I'll be glad to review after that.

tdejager · 2025-09-18T14:39:11Z

@remimimimimi Yeah sorry about that, because of ignore we were using gitignore rules instead of unix glob, so had to change some stuff here and there.

Edit:
More failures..
Should fixed the windows failure but pixi build I need to look more closely.

tdejager · 2025-09-18T14:39:39Z

crates/pixi_command_dispatcher/src/source_build_cache_status/mod.rs

-/// A list of globs that should be ignored when calculating any input hash.
-/// These are typically used for build artifacts that should not be included in
-/// the input hash.
-pub const DEFAULT_BUILD_IGNORE_GLOBS: &[&str] = &["!.pixi/**"];


Dont need this we now exclude hidden by default

This kind of feels wrong to me, but I can't articulate why.

We for sure should document this though 🙂

This is a switch I can flip. But I feel you also don't want to hover something like the .git folder by default.

Yeah, I couldn't think of any case where you'd want a folder or file starting with . to be included.
Let's just document this for now :)

tdejager · 2025-09-18T14:40:16Z

crates/pixi_glob/src/glob_set/mod.rs

@@ -0,0 +1,216 @@
+//! Convenience wrapper around `ignore` that emulates the glob semantics pixi expects.
+//!
+//! Notable behavioural tweaks compared to vanilla gitignore parsing, so that it behaves more like unix globbing with special rules:


This was all a bit hairy to get it to behave like expected. I've made codex explain that part here.

tdejager · 2025-09-18T14:41:37Z

Because we are using custom special globbing semantics, it might start to make sense to explain this in the docs explicitly in a follow up PR.

tdejager · 2025-09-18T14:59:06Z

Hmm will put it into draft until I figure out the pixi build test.

nichmor

I've tested it and checkout the code - looks very good!

some small ideas - maybe we can rename glob_walk_root into walk_root as it's already in the glob_set module.

Apart from that I like it

perf: ignore instead of wax

805735d

Merge branch 'main' into perf/ignore-instead-of-wax

9bba7eb

feat: use ignore for the glob walk on the hash checking

cead6f7

tdejager added 4 commits September 16, 2025 14:33

wip: working on full rewrite to ignore

e0d0a87

fix: clippy

539c305

feat: some cleanup

6b94f88

fix: typo

be3923b

lucascolley added the performance An issue related to performance label Sep 17, 2025

tdejager and others added 5 commits September 17, 2025 20:37

wip: new walking algorithm

e78c731

feat: snapshots in new format

4fd0afe

final touches

fa18ecb

feat: rename back to globset

fbb1871

Merge branch 'main' into perf/ignore-instead-of-wax

7148d94

tdejager marked this pull request as ready for review September 18, 2025 11:43

tdejager requested review from nichmor and remimimimimi September 18, 2025 11:59

tdejager commented Sep 18, 2025

View reviewed changes

feat: hidden is now default

4b06c06

tdejager added 3 commits September 18, 2025 16:12

feat: works with tasks now

1fe695c

feat: remove dbg

4694534

feat: add more clarification in the comments

b34f552

tdejager commented Sep 18, 2025

View reviewed changes

feat: fix seperator windows

2a031b7

tdejager marked this pull request as draft September 18, 2025 14:59

tdejager mentioned this pull request Sep 19, 2025

feat: change glob types prefix-dev/pixi-build-backends#368

Merged

nichmor added 2 commits September 23, 2025 10:44

Merge branch 'main' into perf/ignore-instead-of-wax

968ee85

misc: add docstrings for struct fields

70bd699

nichmor marked this pull request as ready for review September 23, 2025 13:52

nichmor added 2 commits September 23, 2025 16:52

Merge branch 'main' into perf/ignore-instead-of-wax

027d62c

misc: update lockfile

3bf7a2a

nichmor approved these changes Sep 23, 2025

View reviewed changes

tdejager and others added 3 commits September 24, 2025 09:16

fix: rename

1e8591d

Merge branch 'main' into perf/ignore-instead-of-wax

1234070

feat: add lock again

a723ff4

tdejager merged commit 43b0cbf into prefix-dev:main Sep 25, 2025
38 checks passed

ruben-arts mentioned this pull request Oct 2, 2025

perf: make globbing more parallel #4548

Closed

BrewTestBot mentioned this pull request Oct 7, 2025

pixi 0.56.0 Homebrew/homebrew-core#247185

Merged

		@@ -0,0 +1,437 @@
		//! Plan the effective glob walk root for a set of patterns that may contain relative components.

perf: ignore instead of wax #4578

perf: ignore instead of wax #4578

Uh oh!

Conversation

tdejager commented Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Update 18-09:

Why?

Description

What changed?

How this was tested.

Review

Uh oh!

ruben-arts commented Sep 12, 2025

Uh oh!

tdejager commented Sep 12, 2025

Uh oh!

ruben-arts commented Sep 12, 2025

Uh oh!

Hofer-Julian commented Sep 12, 2025

Uh oh!

tdejager commented Sep 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

remimimimimi commented Sep 18, 2025

Uh oh!

tdejager commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tdejager commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tdejager commented Sep 18, 2025

Uh oh!

nichmor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

tdejager commented Sep 12, 2025 •

edited

Loading

tdejager commented Sep 18, 2025 •

edited

Loading

tdejager commented Sep 18, 2025 •

edited

Loading