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

Use syncpool for chunk buffer #775

Merged
merged 1 commit into from
Sep 5, 2024
Merged

Conversation

tobbe76
Copy link
Contributor

@tobbe76 tobbe76 commented Sep 2, 2024

This reduces the number of gc:s and improves performance.
I have tested this change on our production server with good result.

Copy link
Collaborator

@mostynb mostynb left a comment

Choose a reason for hiding this comment

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

I wonder if we could simplify this to just return slices, instead of pointers to slices?

cache/disk/casblob/casblob.go Show resolved Hide resolved
cache/disk/casblob/casblob.go Show resolved Hide resolved
@tobbe76
Copy link
Contributor Author

tobbe76 commented Sep 5, 2024

I have simplified it now. And the number of gc allocations is still the same in my stress test.

The reason why i used a ptr was from this thread, but to my understanding now when reading it again, it's a very small optimization using a ptr.
golang/go#16323

@mostynb
Copy link
Collaborator

mostynb commented Sep 5, 2024

Hmm, looks like there's a lint failure that wants us to use pointer types after all: https://staticcheck.dev/docs/checks#SA6002

Sorry- can you change that part back (and drop the third argument to make)?

This reduces the number of gc:s and improves performance.
@mostynb mostynb merged commit 03892f8 into buchgr:master Sep 5, 2024
3 checks passed
@mostynb
Copy link
Collaborator

mostynb commented Sep 5, 2024

Landed this change, though I removed the nil check in the defer statement (it seems unnecessary). Thanks again.

BTW, I notice that you're also in Göteborg according to github :) I wonder if you could share some approximate details of your bazel-remote install? I'm always looking for feedback on how people are using it.

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