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

fix: slightly reduce memory usage when walking large directory trees #45

Merged
merged 1 commit into from
Aug 1, 2019

Conversation

Stebalien
Copy link
Member

This adds up quick.

@Stebalien Stebalien merged commit 654ee69 into master Aug 1, 2019
@Stebalien Stebalien deleted the fix/walk-memory-usage branch August 1, 2019 20:14
@lanzafame
Copy link

@Stebalien I know we don't currently have much benchmarking infra in place but I would love to see small 'obvious' perf improvements like this have a benchmark added or at the least the output of a benchmark before/after. I know in this case that it is an obvious win but not every case is that way and we don't have a practice of confirming our perf improvements on the 'function' level.

@Stebalien
Copy link
Member Author

For cases like this, I confirm by re-running the daemon on a known workload and checking. This is one of the reasons I want to get pprof profiles from benchmarks. Given a predictable workload, we can easily detect memory usage regressions by looking at the top ten allocators.

@lanzafame
Copy link

I confirm by re-running the daemon on a known workload and checking

That only helps you contribute perf improvements though 😄

@Stebalien
Copy link
Member Author

Ideally, we'd have something continuously running common workloads and taking profiles. But that's mostly useful for catching regressions. When making improvements, profiling is really the only way to go.

Jorropo pushed a commit to ipfs/boxo that referenced this pull request Mar 15, 2023
fix: slightly reduce memory usage when walking large directory trees

This commit was moved from ipfs/go-merkledag@654ee69
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