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

Checkpoint only after Update; Keep records in the sync.Map longer #647

Merged
merged 7 commits into from
Apr 22, 2020

Conversation

jmacd
Copy link
Contributor

@jmacd jmacd commented Apr 19, 2020

Removes TODOs left from #468. An atomic increment is carried out for each Update(), allowing the collection logic to checkpoint records only when they have actually received updates (2 TODOs). This was mentioned in the review for #641.

This allows an optimization that had been in place before #468, that is to defer unmapping a record until it has survived a whole collection interval without being updated (1 TODO). This ensures that users of the direct calling convention that happen to repeatedly use the same label set will benefit from cached label encodings.

BEFORE:

BenchmarkRepeatedDirectCalls-8   	 1217017	      1001 ns/op	     304 B/op	       7 allocs/op

AFTER:

BenchmarkRepeatedDirectCalls-8   	 2575458	       454 ns/op	     240 B/op	       3 allocs/op

@jmacd jmacd added the area:metrics Part of OpenTelemetry Metrics label Apr 19, 2020
@jmacd jmacd requested a review from bogdandrutu April 19, 2020 05:58
sdk/metric/sdk.go Outdated Show resolved Hide resolved
sdk/metric/correct_test.go Show resolved Hide resolved
@krnowak
Copy link
Member

krnowak commented Apr 21, 2020

Looks good, but there is a conflict to resolve.

@jmacd jmacd merged commit 395440d into open-telemetry:master Apr 22, 2020
@jmacd jmacd deleted the jmacd/keepinmap branch April 22, 2020 03:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metrics Part of OpenTelemetry Metrics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants