Skip to content
This repository has been archived by the owner on May 23, 2024. It is now read-only.

Avoid defer when generating random number #358

Merged
merged 1 commit into from
Dec 12, 2018

Conversation

objectiser
Copy link
Contributor

Signed-off-by: Gary Brown [email protected]

Which problem is this PR solving?

Remove defer used in generating random number, as recommended by @mandarjog here: a9d03b8#r31655784

Short description of the changes

Instead of putting generator back in pool using defer, do it explicitly.

@ghost ghost assigned objectiser Dec 12, 2018
@ghost ghost added the review label Dec 12, 2018
@objectiser
Copy link
Contributor Author

cc @mandarjog

@codecov
Copy link

codecov bot commented Dec 12, 2018

Codecov Report

Merging #358 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #358      +/-   ##
==========================================
+ Coverage   87.22%   87.22%   +<.01%     
==========================================
  Files          54       54              
  Lines        3005     3006       +1     
==========================================
+ Hits         2621     2622       +1     
  Misses        272      272              
  Partials      112      112
Impacted Files Coverage Δ
tracer.go 90.13% <100%> (+0.04%) ⬆️

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 a9d03b8...9bdf03d. Read the comment docs.

Copy link
Contributor

@black-adder black-adder left a comment

Choose a reason for hiding this comment

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

do benchmarks show an improvment without the defer?

@objectiser
Copy link
Contributor Author

@black-adder No real difference, but was a small test. However there seems to be evidence that defer is slower.

@black-adder black-adder merged commit 3e70059 into jaegertracing:master Dec 12, 2018
@ghost ghost removed the review label Dec 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants