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

Slight performance overhead can be saved by using Random.nextDouble i… #457

Closed
wants to merge 2 commits into from

Conversation

@bd2019us
Copy link

@bd2019us bd2019us commented May 10, 2019

…nstead of Math.random for any instance of Math.random.

Description

Though this is not the only instance of Math.random in the codebase, any instance of it can be replaced by Random.nextDouble. Though not performance critical, this can reduce performance overhead when done across all instances of Math.random.

  • Added import for java.util.Random
  • Replaced the instance of Math.random with Random.nextDouble

I have a list of every applicable Math.random -> Random.nextDouble instance and will share such list if it is okay to move forward with such modifications.

Motivation and Context

Though not performance critical, this can reduce performance overhead when done across all instances of Math.random.

Types of changes

  • Reduced overhead in performance

Checklist:

  • [ x ] My code follows the code style of this project.
  • [ x ] I have updated the documentation accordingly. (Documentation did need to be updated)
bd2019us added 2 commits May 10, 2019
@codecov-io
Copy link

@codecov-io codecov-io commented May 10, 2019

Codecov Report

Merging #457 into trunk will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##              trunk     #457      +/-   ##
============================================
+ Coverage     61.19%   61.19%   +<.01%     
- Complexity    11462    11463       +1     
============================================
  Files          1220     1220              
  Lines         77974    77975       +1     
  Branches       7527     7527              
============================================
+ Hits          47717    47719       +2     
+ Misses        27556    27555       -1     
  Partials       2701     2701
Impacted Files Coverage Δ Complexity Δ
...s/org/apache/jmeter/timers/PoissonRandomTimer.java 73.68% <100%> (+0.71%) 9 <0> (ø) ⬇️
...c/core/org/apache/jmeter/reporters/Summariser.java 86.25% <0%> (+0.76%) 19% <0%> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f69f66e...f4e1cf9. Read the comment docs.

@asfgit asfgit closed this in da64e05 May 12, 2019
@FSchumacher
Copy link
Contributor

@FSchumacher FSchumacher commented May 12, 2019

Thanks for your contribution.
I have used ThreadLocalRandom as it should be even better performing, when multiple threads are using the timer.

StorDm pushed a commit to etnetera/jmeter that referenced this issue Jan 7, 2021
…ocalRandom.

Based on a patch by Xia Li.

Closes apache#457


git-svn-id: https://svn.apache.org/repos/asf/jmeter/trunk@1859133 13f79535-47bb-0310-9956-ffa450edef68

Former-commit-id: da64e05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants