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

[tsm1] Change timestamp from time.Time to unixnano #5522

Merged
merged 4 commits into from
Feb 4, 2016

Conversation

methane
Copy link
Contributor

@methane methane commented Feb 3, 2016

time.Time contains timezone loc *Location.
Since tsm1.Cache holds a lot of values in heap, there are many pointers in heap.
This makes GC heavy.

before:
$ go test -run=xxx -bench=BenchmarkCache
PASS
BenchmarkCacheFloatEntries-4         300       4019495 ns/op

after:
$ go test -run=xxx -bench=BenchmarkCache
PASS
BenchmarkCacheFloatEntries-4         500       3720449 ns/op

related issues:


  • CHANGELOG.md updated
  • Rebased/mergable
  • Tests pass
  • Sign CLA (if not already signed)

@jwilder jwilder added this to the 0.11.0 milestone Feb 4, 2016
@jwilder
Copy link
Contributor

jwilder commented Feb 4, 2016

Thanks @methane. I see similar improvements locally. There are other places where time.Time is used and a change like this might help them as well.

influxdb:influxdb jason$ go test -run non -bench BenchmarkCacheFloatEntries -benchmem ./tsdb/engine/tsm1/
PASS
BenchmarkCacheFloatEntries       500       3368979 ns/op      593239 B/op      30016 allocs/op
influxdb:influxdb jason$ go test -run non -bench BenchmarkCacheFloatEntries -benchmem ./tsdb/engine/tsm1/
PASS
BenchmarkCacheFloatEntries       500       3120040 ns/op      433206 B/op      30015 allocs/op

@methane
Copy link
Contributor Author

methane commented Feb 4, 2016

I've seen profiler output posted on golang/go#14189.
I found most of memory is consumed by this cache.
So this is the largest part of memory consumption and GC time.

@e-dard
Copy link
Contributor

e-dard commented Feb 4, 2016

LGTM 👍

jwilder added a commit that referenced this pull request Feb 4, 2016
[tsm1] Change timestamp from time.Time to unixnano
@jwilder jwilder merged commit f6ebb0b into influxdata:master Feb 4, 2016
@methane
Copy link
Contributor Author

methane commented Feb 5, 2016

@toddboom Would you run the benchmark same to golang/go#14189 ?

@methane methane deleted the tsm1-optimize branch February 12, 2016 18:52
@jwilder jwilder mentioned this pull request Feb 25, 2016
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants