Crockeo
Crockeo
Replied to the comments above. Thanks y'all for taking the time to review this PR!
@googlebot I signed it!
> I don't love that it involves getting a new dependency into the stat lib, I didn't like having envconfig there either. I was hoping we can get to zero....
>this is obviously not a way the library is intended to be used - was this fixed in the service Agreed that this isn't the intended use case, and yes...
>I tried running the benchmarks for a comparison, but BenchmarkStoreNewPerInstanceCounter panicked. Looks like I missed some test cases initializing `statStore` with a zero-value. E.g. for the benchmark you mentioned: https://github.com/lyft/gostats/blob/e6b88e67ba70cb8f1dbc2dcc020f779b3e17ab3e/stats_test.go#L471...
range thing looks like it does a good job at bringing down contention: ``` this PR: BenchmarkStore_MutexContention-10 4006423 297.9 ns/op with range: BenchmarkStore_MutexContention-10 16804858 69.96 ns/op original: BenchmarkStore_MutexContention-10 20921458 56.88...
closing this because it has conflicts and i don't have the time to fix them now-a-days. thanks y'all for taking a look!