Skip to content

Conversation

bboreham
Copy link
Collaborator

@bboreham bboreham commented Jul 11, 2018

They can be trivially computed when required.
Smaller structs = less garbage-collection = better performance.

This is a breaking change in the wire protocol; will need a version bump. New app will work with older probes, just ignoring the data.

I checked the JavaScript and can only find one use of the field, and that already falls back to the alternative.

Benchmarks (unmarshalling older reports which do contain the fields)

Before:

BenchmarkReportUnmarshal-2   	       1	1090111087 ns/op	184140088 B/op	 2497821 allocs/op
BenchmarkReportMerge-2   	       3	 437993382 ns/op	110116245 B/op	  887843 allocs/op

After:

BenchmarkReportUnmarshal-2   	       1	1089826095 ns/op	172437016 B/op	 2497790 allocs/op
BenchmarkReportMerge-2   	       3	 397821741 ns/op	102478133 B/op	  887843 allocs/op

The impact on Merge() is more noticeable when combined with #3253:

BenchmarkReportMerge-2   	       5	 303941149 ns/op	56328974 B/op	  476558 allocs/op

They can be trivially computed when required.
@bboreham bboreham added the performance Excessive resource usage and latency; usually a bug or chore label Jul 11, 2018
}

// Following two functions are exported for testing only: make sure there are samples before calling.

This comment was marked as abuse.

They were exported for testing only, and the test isn't very useful.
@bboreham bboreham mentioned this pull request Jul 19, 2018
@bboreham bboreham merged commit c1b1ee2 into master Aug 24, 2018
@bboreham bboreham deleted the slim-metrics branch January 13, 2020 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Excessive resource usage and latency; usually a bug or chore
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants