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

pkg/fuzzer: pregenerate gen/fuzz requests #4612

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dvyukov
Copy link
Collaborator

@dvyukov dvyukov commented Mar 28, 2024

Mutation can be quite slow and it's on the critical ExchangeInfo path.
Pregenerate gen/fuzz requests in a separate goroutine,
so that they are readily available for consumption most of the time.

Mutation can be quite slow and it's on the critical ExchangeInfo path.
Pregenerate gen/fuzz requests in a separate goroutine,
so that they are readily available for consumption most of the time.
@dvyukov dvyukov requested a review from a-nogikh March 28, 2024 18:11
Copy link

codecov bot commented Mar 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.0%. Comparing base (eaa9ee9) to head (359d28c).
Report is 1 commits behind head on master.

Additional details and impacted files
Files Coverage Δ
pkg/fuzzer/fuzzer.go 82.2% <100.0%> (+1.3%) ⬆️

... and 1 file with indirect coverage changes

@dvyukov
Copy link
Collaborator Author

dvyukov commented Mar 28, 2024

It looks like a single goroutine can't keep up pregenerating inputs:
image

I think we need to start NumCPU goroutines + pre-serialize programs in Requests, so that Exchange can just take the byte slice and send.

@a-nogikh
Copy link
Collaborator

Did you observe any performance improvements?

@a-nogikh
Copy link
Collaborator

a-nogikh commented Apr 2, 2024

And you tested it with the non-instrumented kernel on VMs, right? I think when a kernel is instrumented, and especially when we're on Cloud and have extra network-related delays, the CPU time spent on gen/fuzz operations won't be that much of a bottleneck.

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.

None yet

2 participants