-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
[fuzz] Made health check fuzz more efficient #13747
Conversation
Signed-off-by: Zach <zasweq@google.com>
/assign @asraa @adisuissa @htuch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
/wait
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General question about the memory allocations throughout the code:
Is there any code that handles the deallocation, or is it just deallocated when the test finishes?
In a unit test setting, the tests are somewhat short so it might be ok, but for fuzz tests if this is called multiple times and it could use too much memory.
What do you think?
expectClientCreate(0); | ||
} | ||
|
||
void GrpcHealthCheckFuzz::expectClientCreate(size_t index) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the given index
argument always 0?
If that's the case should we keep this argument, or just remove?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup! Asra mentioned that as well :). Removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In regards to your deallocation question, there is no explicit code that deallocates memory. I don't think the memory scales with each fuzz iteration though, as it gets destructed each time.
Signed-off-by: Zach <zasweq@google.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but did you consider any options for reducing the amount of boilerplate copy+paste here? No need for gymnastics, but if it's possible it would be nice.
Signed-off-by: Zach <zasweq@google.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Interested to merge and see speedups.
/wait
Signed-off-by: Zach <zasweq@google.com>
* master: (83 commits) tls: Typesafe tls slots (envoyproxy#13789) docs(example): Correct URL for caching example page (envoyproxy#13810) [fuzz] Made health check fuzz more efficient (envoyproxy#13747) rtds: properly scope rtds stats (envoyproxy#13764) http: fixing a bug with IPv6 hosts (envoyproxy#13798) connection: Remember transport socket read resumption requests and replay them when re-enabling read. (envoyproxy#13772) network: adding some accessors for ALPN work. (envoyproxy#13785) docs: added a step about how to handle platform specific extensions (envoyproxy#13759) Fix identation in ip transparency code snippet (envoyproxy#13743) wasm: enable WAVM's stack unwinding feature (envoyproxy#13792) log: set route name for direct response (envoyproxy#13683) Use nghttp2 as external dependsncy in protocol_constraints_lib (envoyproxy#13763) [Windows] Update windows dev docs (envoyproxy#13741) cel: patch thread safety issue (envoyproxy#13739) Windows: Fix ssl_socket_test (envoyproxy#13264) apple dns: add fake api test suite (envoyproxy#13780) overload: scale selected timers in response to load (envoyproxy#13475) examples: Add dynamic configuration (control plane) sandbox (envoyproxy#13746) Removed exception in getResponseStatus() (envoyproxy#13314) network: add timeout for transport connect (envoyproxy#13610) ... Signed-off-by: Michael Puncel <mpuncel@squareup.com>
Signed-off-by: Zach Reyes zasweq@google.com
Commit Message: Made health check fuzz more efficient.
Additional Description: Orthogonal with the same logic as my host/mentor Asra's PR #13729. Took away test session vector for gRPC health checking, as it only ever used one. Also made mock timers in TCP and gRPC NiceMocks to take away uninteresting call logs, which were slowing down the health check fuzzer. Speeds up from 5 -> 30 exec/sec locally.
Risk Level: Low