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

avoid using interface{} in Packet #81

Merged
merged 2 commits into from May 16, 2016
Merged

Conversation

ploxiln
Copy link
Contributor

@ploxiln ploxiln commented Apr 28, 2016

This is at least partly a subjective thing, which I tried mostly as an experiment, and I offer up in case there is agreement that it's not a bad idea:

instead of Value interface{}, have ValFlt float64, ValStr string

  • may prevent some runtime type cast failures
  • may slightly improve performance
[pierce@plo-pro statsdaemon]$ go test -bench=Pa -run=none -benchtime 2s
PASS
BenchmarkParseLineCounter-4      2000000              1414 ns/op
BenchmarkParseLineGauge-4        2000000              1441 ns/op
BenchmarkParseLineTimer-4        2000000              1441 ns/op
BenchmarkParseLineSet-4          2000000              1361 ns/op
BenchmarkPacketHandlerCounter-4 20000000               134 ns/op
BenchmarkPacketHandlerGauge-4   20000000               152 ns/op
BenchmarkPacketHandlerTimer-4   20000000               181 ns/op
BenchmarkPacketHandlerSet-4     20000000               192 ns/op
ok      _/Users/pierce/scratch/statsdaemon      31.059s
[pierce@plo-pro statsdaemon]$ git checkout -
Switched to branch 'packet_no_interface'
[pierce@plo-pro statsdaemon]$ go test -bench=Pa -run=none -benchtime 2s
PASS
BenchmarkParseLineCounter-4      2000000              1360 ns/op
BenchmarkParseLineGauge-4        2000000              1333 ns/op
BenchmarkParseLineTimer-4        2000000              1394 ns/op
BenchmarkParseLineSet-4          2000000              1314 ns/op
BenchmarkPacketHandlerCounter-4 20000000               120 ns/op
BenchmarkPacketHandlerGauge-4   20000000               130 ns/op
BenchmarkPacketHandlerTimer-4   20000000               167 ns/op
BenchmarkPacketHandlerSet-4     20000000               174 ns/op
ok      _/Users/pierce/scratch/statsdaemon      28.837s

instead of Value interface{}, have ValFlt float64, ValStr string

may prevent some runtime type cast failures
may slightly improve performance
separate benchmarks for parsing and handling each type of metric
@mreiferson
Copy link
Contributor

I'm +1 and a fan in general when there are known controlled types.

@ploxiln
Copy link
Contributor Author

ploxiln commented May 16, 2016

For what it's worth: I've been running this commit, which includes all of master, in production for about a week. It's been working.

Might be time for a release with support for floats 😁

@jehiah jehiah merged commit 481ea9d into bitly:master May 16, 2016
@ploxiln ploxiln deleted the packet_no_interface branch November 1, 2016 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants