[PoC] Use timestamp_delta to weight samples#166
Open
casperisfine wants to merge 1 commit intotmm1:masterfrom
Open
[PoC] Use timestamp_delta to weight samples#166casperisfine wants to merge 1 commit intotmm1:masterfrom
casperisfine wants to merge 1 commit intotmm1:masterfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NB: this is just proof of concept quality to start a discussion. A releasable version of this patch would involve quite more work.
Problem
We recently noticed that we were overlooking a major hotspot because stackprof regular reporting is biased.
Here's the stackprof summary of one of our application boot, using
stackprof 0.2.17:And now here's the same profiling data, but rendered by Speedscope:
Notice how
Module#usingaccount for almost nothing according to Stackprof, but for 11% for speedscope.This is because stackprof's own reporting is based on the number of samples, whereas Speedscope use the
raw_timestamp_deltasdata that is intended for flamegraphs.And since
Module#usinggoes over the whole heap to flush method caches, I assume that the stackprof event isn't fired for a long amount of time. Based on the stackprof data,Module#usingwas sampled311times, but each sample took41msinstead of1ms.This is particularly visible for
Module#using, but from analyzingraw_timestamp_deltas, a large proportion of the samples are longer than intended.Possible solution
Again this is just a proof of concept, but if we were to always compute the
timestamp_delta, we could "weight" the samples to somewhat correct this bias.It's a bit hard to implement because of other profiling mode such as
objectwhich aren't time based. So maybe instead of weighting the samples, we should simply record thetimestamp_deltaas part of the profiling, and use that instead of the sample count for reporting.@tenderlove @XrXr what do you think?