Skip to content

Relax benchmark commandline restriction#243

Merged
zhengyu123 merged 2 commits intomainfrom
zgu/jmh-arguments
Jul 11, 2025
Merged

Relax benchmark commandline restriction#243
zhengyu123 merged 2 commits intomainfrom
zgu/jmh-arguments

Conversation

@zhengyu123
Copy link
Contributor

@zhengyu123 zhengyu123 commented Jul 8, 2025

What does this PR do?:
Allow additional jmh arguments, e.g. profiling option

Motivation:
Run jmh benchmark with profiler, e.g.

java -jar ddprof-stresstest.jar throughput.ThreadFilterBenchmark -prof perfasm

Additional Notes:

How to test the change?:

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles
    credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.
  • JIRA: [JIRA-XXXX]

Unsure? Have a question? Request a review!

@zhengyu123 zhengyu123 requested review from jbachorik and r1viollet July 8, 2025 19:00
@github-actions
Copy link
Contributor

github-actions bot commented Jul 8, 2025 •
edited
Loading

🔧 Report generated by pr-comment-cppcheck

CppCheck Report

Errors (2)

Warnings (4)

Style Violations (301)

@github-actions
Copy link
Contributor

github-actions bot commented Jul 8, 2025 •
edited
Loading

🔧 Report generated by pr-comment-scanbuild

r1viollet
r1viollet approved these changes Jul 9, 2025
View reviewed changes
Copy link
Contributor

@r1viollet r1viollet left a comment

Choose a reason for hiding this comment

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

LGTM, though it would still be nice to offer documentation or a help text to users.

jbachorik
jbachorik reviewed Jul 9, 2025
View reviewed changes
Copy link
Collaborator

@jbachorik jbachorik 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, could you go the extra mile and completely switch to the JMH standard arguments?
Eg. using the JMH command line argument for 'include' instead of hardcoding it in the launcher?

@zhengyu123
Copy link
Contributor Author

zhengyu123 commented Jul 10, 2025

I wonder, could you go the extra mile and completely switch to the JMH standard arguments? Eg. using the JMH command line argument for 'include' instead of hardcoding it in the launcher?

Not sure this is what you want. This matches my normal usage pattern, e.g.

java -jar ddprof-stresstest.jar com.datadoghq.profiler.stresstest.scenarios.counters.ThreadFilterBenchmark -prof com.datadoghq.profiler.stresstest.WhitboxProfiler

@zhengyu123 zhengyu123 merged commit f5bb495 into main Jul 11, 2025
93 of 95 checks passed
@zhengyu123 zhengyu123 deleted the zgu/jmh-arguments branch July 11, 2025 16:37
@github-actions github-actions bot added this to the 1.29.0 milestone Jul 11, 2025
@zhengyu123 zhengyu123 mentioned this pull request Jul 28, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jbachorik jbachorik jbachorik approved these changes

@r1viollet r1viollet r1viollet approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.29.0

Development

Successfully merging this pull request may close these issues.

3 participants

@zhengyu123 @jbachorik @r1viollet