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

globwalker: avoid spurious allocations in iterator #38

Merged
merged 1 commit into from
Jan 6, 2024

Conversation

lucab
Copy link
Contributor

@lucab lucab commented Jan 6, 2024

This tweaks the GlobWalker iterator logic in order to avoid a spurious allocation for each processed directory entry (including ignored ones).

Verified by running the current testsuite under heaptrack and observing allocations stats:

$ heaptrack target/debug/deps/globwalk-baseline
[...]
heaptrack stats:
        allocations:            7176
[...]

$ heaptrack target/debug/deps/globwalk-patched
[...]
heaptrack stats:
        allocations:            7081
[...]

This tweaks the `GlobWalker` iterator logic in order to avoid
a spurious allocation for each processed directory entry (including
ignored ones).

Verified by running the current testsuite under `heaptrack` and
observing allocations stats:

```
$ heaptrack target/debug/deps/globwalk-baseline
[...]
heaptrack stats:
        allocations:            7176
[...]

$ heaptrack target/debug/deps/globwalk-patched
[...]
heaptrack stats:
        allocations:            7081
[...]
```
@Gilnaa Gilnaa merged commit 7c4465b into Gilnaa:master Jan 6, 2024
4 of 6 checks passed
@Gilnaa
Copy link
Owner

Gilnaa commented Jan 6, 2024

Neat! Thanks

@lucab
Copy link
Contributor Author

lucab commented Jan 6, 2024

Thanks for the quick merge!
For context, I came to this while looking into the allocation profile of https://github.com/getzola/zola (through its https://github.com/Keats/tera dependency).
It would be great if you could tag a bugfix 0.9.1 release with this, whenever you have time, so that I can pick up the improvement there.
Thanks again for maintaining this crate 🎉

@lucab lucab deleted the ups/globwalker-next-path-ref branch January 6, 2024 20:16
@Gilnaa
Copy link
Owner

Gilnaa commented Jan 6, 2024

No problem, published now.
Do note that tera is probably going to stay on globwalk 0.8 for now;
I admit I'm not well-versed with how the resolver works, but you might need to force its hand.

Purely out of curiosity, can I ask you to spare a bit more details on what you tried to solve?
How much of an effect did those allocations have?

@lucab
Copy link
Contributor Author

lucab commented Jan 8, 2024

Do note that tera is probably going to stay on globwalk 0.8 for now

Thanks for the heads-up, I guess I'm going to wait for the next major version then.

Purely out of curiosity, can I ask you to spare a bit more details on what you tried to solve?
How much of an effect did those allocations have?

I just wanted to have a look at zola build CPU/memory usage patterns and noticed this low-hanging fruit.
Impact is pretty limited but it linearly grows with the number of files. On the CI testsite (small-medium size, ~150 files) it accounts for ~1% of temporary allocations (allocations directly followed by their deallocation).

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