-
Notifications
You must be signed in to change notification settings - Fork 11
Avoid calling profiler.stop() twice in ThreadFilterBenchmark #257
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
Conversation
| } | ||
|
|
||
| // Stop the profiler if it's active | ||
| try { |
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.
better to stop in tear down or setup in case it is still running ?
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.
Not against that, but the framework (WhiteboxProfiler) handles it already.
r1viollet
left a comment
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.
I had both in case one was not called for some reason.
I'm not sure which makes more sense to remove.
I don't think the exception is a huge deal, though I'm fine removing either stops.
The exception prevents jmh profiler (e.g. prefasm) from generating output. |
What does this PR do?:
Avoid
IllegalStateExceptionexception due to double stop, which can result no output from jmh profiler.Motivation:
Cleanup and make benchmark more robust.
Additional Notes:
How to test the change?:
Run the benchmark,
ThreadFilterBenchmarkno longer throwsIllegalStateExceptionexception.For Datadog employees:
credentials of any kind, I've requested a review from
@DataDog/security-design-and-guidance.Unsure? Have a question? Request a review!