Some LegacyCallStackProfileBuilder samples contain frames from previous stacks |
||
Issue descriptionThe current sample is being cleared only in cases where the stack is not deduplicated. It should be cleared in both deduplicated and non-deduplicated cases. This is resulting in frames from previous deduplicated samples incorrectly being added to subsequent samples.
,
Nov 14
I suspect it was introduced at some point during the refactoring. I still need to analyze the impact of this bug on the resulting data and should be able to pin it down in that process. I noticed this when analyzing data sizes and was surprised to find that the new profiles were smaller than the old ones. :)
,
Nov 14
Oops, sorry about that :-)
,
Nov 14
No worries. This ideally would have been caught by server-side alerting, but we don't have that in place yet. It looks like this was introduced in 70.0.3523.0 with e91f6fdb3d1b31b3c9ca8370fcab83a290118358. The effect is that 9% of non-idle samples were treated as idle[1]. These were almost exclusively samples seen in work done from the message loop, so this didn't significantly affect the pre-message loop startup measurement which is substantially what we care about for startup regression detection. To the extent this introduced bias -- beyond just missing non-idle samples -- it would have been against non-idle samples seen immediately after idle samples. [1] https://uma.googleplex.com/p/chrome/callstacks?sid=812677caa3d4e349dee574dd82af7515 demonstrates this shift pretty clearly.
,
Nov 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/311a3e55aa4ddc50db1049b6af0776b94d77517a commit 311a3e55aa4ddc50db1049b6af0776b94d77517a Author: Mike Wittman <wittman@chromium.org> Date: Wed Nov 14 23:12:35 2018 [Sampling profiler] Fix incorrect reuse of frames across samples Frames from deduplicated samples were being left in the current sample state on the class, resulting in incorrect extra frames being added when the next sample was recorded. This change eliminates the current sample state to avoid this problem, and adds a test for this case. Bug: 905379 Change-Id: I5fe0ccd3e50675669d4c7124fb7f64ebb85fe807 Reviewed-on: https://chromium-review.googlesource.com/c/1336247 Reviewed-by: Xi Cheng <chengx@chromium.org> Commit-Queue: Mike Wittman <wittman@chromium.org> Cr-Commit-Position: refs/heads/master@{#608160} [modify] https://crrev.com/311a3e55aa4ddc50db1049b6af0776b94d77517a/components/metrics/legacy_call_stack_profile_builder.cc [modify] https://crrev.com/311a3e55aa4ddc50db1049b6af0776b94d77517a/components/metrics/legacy_call_stack_profile_builder.h [modify] https://crrev.com/311a3e55aa4ddc50db1049b6af0776b94d77517a/components/metrics/legacy_call_stack_profile_builder_unittest.cc
,
Nov 19
The fix went in to 72.0.3611.0 and the generated profiles from that version look good. |
||
►
Sign in to add a comment |
||
Comment 1 by chengx@chromium.org
, Nov 14