Mesure profiles that contain no callstack samples using heuristic - #3698
Mesure profiles that contain no callstack samples using heuristic#3698maciejburda wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3698 +/- ##
===========================================
+ Coverage 73.41% 73.45% +0.04%
===========================================
Files 999 1000 +1
Lines 36565 36642 +77
Branches 6181 6202 +21
===========================================
+ Hits 26842 26912 +70
+ Misses 7998 7995 -3
- Partials 1725 1735 +10
🚀 New features to boost your workflow:
|
Overall question what is this come from? if this is not a specification from Perfetto format but a random difference which can be changed during the Perfetto iteration, then we are sitting on a brittle workaround. |
|
That's a good point! Hope is that we can remove this fix once we observe ProfilingManagerAPI fix fully roll out. |
@maciejburda I need to confirm with the Perfetto team about it before merging since it doesn't come from a valid source, the risk is that it can also reject the non-empty profiles if this can vary from profiles. |
|
Just realized that I didn't open as draft. I'm still working on this. That's a first iteration. I want to cut the changes short. I went out of line with the amount of lines. |
924d948 to
159341b
Compare
Some captures come back with the profiler having run at full cadence for the whole window and produced no perf_sample at all. The one that prompted this is 1.19 MiB of payload-free packets over 57s, so neither file size nor packet count tells it apart from a healthy capture. PerfSampleProbe walks the trace framing and reports whether any TracePacket.perf_sample is present. ProfilingDataWriter reports the verdict and the trace size on the profiling write metric, on the path where the profile is actually uploaded, so the SDK count of empty captures lines up with the count the backend derives on its own. Agreement validates the walk, and the size distribution gives the largest empty profile seen in the field. Nothing is dropped. Only a verdict of no_samples would ever justify dropping, and that decision waits until the rate has been observed, following the same observe-then-enforce order the clock-drift gate used in this file. Temporary, and one file to delete once the platform stops producing these. Validated against two complete unmodified captures, checked in as the archives the profiler produced. Their sample counts, 0 and 51304, come from Perfetto's own trace_processor.
159341b to
d2a9f6b
Compare
What does this PR do?
Measures how often a captured profile contains no callstack samples.
PerfSampleProbechecks whether a trace holds anyTracePacket.perf_sample.ProfilingDataWriterreports the answer asperf_sample_verdicton the profiling write metric, together with the trace size, using bytes it has already read — no extra I/O.Nothing is dropped and nothing else changes. The verdict is reported on the path where the profile is actually uploaded, so the SDK's count of empty captures covers the same population as the backend's own empty-profile count. If the two agree, the walk is validated, and
file_sizeon the same row gives the largest empty profile seen in the field.Dropping them comes later, once that comparison holds — the same observe-then-enforce order the clock-drift gate used in this file (
f78e8a0aa, enforcement inb37401a8b).Temporary: one file to delete once the platform stops producing these.
Motivation
Some captures run the profiler at full cadence for the whole window and produce nothing:
perf_sampleEvery packet in the empty one is a 19-byte envelope with no sample inside, and the symbol table is filled in for samples that never arrived. Nothing about its size or cadence looks wrong, so it uploads as a normal profile.
The empty capture is Perfetto v54 / Android 17, the healthy one v50.1 / Android 16, so this looks like a platform regression rather than something the SDK is doing.
Additional Notes
Is field 66 safe to rely on?
The number is. Protobuf field numbers are the wire format and "should never be reused", and Perfetto honours that in
trace_packet.protoby retiring deleted fields rather than recycling them (reserved 55;,reserved 80, 81;,reserved 106;).What is not promised is that samples keep arriving in field 66 — it sits under
== UNSTABLE / CONTEXT-SPECIFIC FIELDS ==in that same file. Measuring first is what turns that from a risk into a check: if the SDK count and the backend count disagree, the premise is wrong and nothing was lost finding out.Compression. Fields 50 and 133 report
COMPRESSED— the samples may be inside a block this doesn't inflate, so that is not an empty profile.Validation. Two real captures are checked in (~970 KB of test resources). Their sample counts, 0 and 51 304, come from Perfetto's own
trace_processorrather than from this code.Unrelated: 59 tests in
ProfilingFeatureTestandProfilingTestalready fail ondevelopunder JDK 21.Review checklist (to be filled by reviewers)