Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Thread safety by using the default source for the RNG #23

Merged
merged 2 commits into from
Apr 20, 2018
Merged

Thread safety by using the default source for the RNG #23

merged 2 commits into from
Apr 20, 2018

Conversation

ridv
Copy link
Contributor

@ridv ridv commented Apr 20, 2018

  • Using custom sources results in the RNG becoming non-thread safe. Using the default source guarantees that multiple threads are able to generate data using faker safely as per the Go docs (https://golang.org/pkg/math/rand/).

@codecov
Copy link

codecov bot commented Apr 20, 2018

Codecov Report

Merging #23 into master will decrease coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #23      +/-   ##
==========================================
- Coverage   97.85%   97.82%   -0.04%     
==========================================
  Files           8        8              
  Lines         373      367       -6     
==========================================
- Hits          365      359       -6     
  Misses          4        4              
  Partials        4        4
Impacted Files Coverage Δ
address.go 100% <100%> (ø) ⬆️
payment.go 100% <100%> (ø) ⬆️
faker.go 95.2% <100%> (+0.02%) ⬆️
internet.go 100% <100%> (ø) ⬆️

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 8c8836d...8d852b6. Read the comment docs.

@agoalofalife
Copy link
Collaborator

@rdelval the tests all passed successfully
good)

@agoalofalife
Copy link
Collaborator

@rdelval have you checked it manually?

@ridv
Copy link
Contributor Author

ridv commented Apr 20, 2018

Yup checked it manually with very heavy use (50 threads running 1000 iterations generating a good amount of data), as well as running with the -race flag.

By the way, I should mention this might change some of the results for the benchmarks. So if that's important, we should do a benchmark test on the same machine of the non-thread safe version vs the thread safe version and make a decision based on data.

@agoalofalife
Copy link
Collaborator

@rdelval excellent, thanks for the contribution)

@agoalofalife agoalofalife merged commit 1e3dfd5 into bxcodec:master Apr 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants