From d2770f7aef723c50800e3ab0f34ab02b07740f50 Mon Sep 17 00:00:00 2001 From: Lalit Maganti Date: Wed, 25 Mar 2026 19:50:34 +0000 Subject: [PATCH] [DNS] perfetto: add a new flag for "stable incremental state" This CL adds a new flag and associated parsing code for marking incremental state as "stable". By stable, we mean that for a given piece of interned data you *never* define the same incremental scoped id with a different value anywhere else in the trace. This property is necessary for us to continue allowing the packet through the central proto reader code instead of just being dropped. The existing behavior is unchanged and works the same as it does today. --- protos/perfetto/trace/perfetto_trace.proto | 28 +++++++++++++++++++ protos/perfetto/trace/trace_packet.proto | 28 +++++++++++++++++++ .../importers/proto/proto_trace_reader.cc | 13 ++++++--- 3 files changed, 65 insertions(+), 4 deletions(-) diff --git a/protos/perfetto/trace/perfetto_trace.proto b/protos/perfetto/trace/perfetto_trace.proto index db033bc14e0..5bb43756214 100644 --- a/protos/perfetto/trace/perfetto_trace.proto +++ b/protos/perfetto/trace/perfetto_trace.proto @@ -18697,6 +18697,34 @@ message TracePacket { // packet with the SEQ_INCREMENTAL_STATE_CLEARED flag has been seen on the // current |trusted_packet_sequence_id|. SEQ_NEEDS_INCREMENTAL_STATE = 2; + + // This packet requires incremental state to be parsed correctly (like + // SEQ_NEEDS_INCREMENTAL_STATE) but the producer guarantees that all + // incremental state on this sequence is "stable": once a value is emitted + // for any piece of incremental state, that value never changes, even + // across SEQ_INCREMENTAL_STATE_CLEARED boundaries. Clears cause + // re-emission of the same values, never new ones. + // + // This means that after packet loss, the trace reader may still process + // these packets because any incremental state that *is* present is + // guaranteed to be correct, even if incomplete. The consumer must be + // prepared to handle missing incremental state lookups. + // + // "Stable" applies to all forms of incremental state including but not + // limited to: + // - Interned data: an iid always maps to the same value. E.g. iid=1 -> + // "malloc" is never re-emitted as iid=1 -> "free" after a clear. + // - TracePacketDefaults: once set, defaults do not change. E.g. + // timestamp_clock_id=6 stays 6 for the entire session. + // - Sequence-scoped clocks: clock snapshots define a clock relationship + // once. After a clear, the same relationship is re-emitted, never a + // new one. + // + // Sequences using this flag MUST NOT use delta encoding for any values + // (e.g. timestamps). Delta encoding is fundamentally incompatible with + // stability because losing a single packet breaks the chain for all + // subsequent values. + SEQ_NEEDS_STABLE_INCREMENTAL_STATE = 4; }; optional uint32 sequence_flags = 13; diff --git a/protos/perfetto/trace/trace_packet.proto b/protos/perfetto/trace/trace_packet.proto index e93b793ab39..002708bfc27 100644 --- a/protos/perfetto/trace/trace_packet.proto +++ b/protos/perfetto/trace/trace_packet.proto @@ -362,6 +362,34 @@ message TracePacket { // packet with the SEQ_INCREMENTAL_STATE_CLEARED flag has been seen on the // current |trusted_packet_sequence_id|. SEQ_NEEDS_INCREMENTAL_STATE = 2; + + // This packet requires incremental state to be parsed correctly (like + // SEQ_NEEDS_INCREMENTAL_STATE) but the producer guarantees that all + // incremental state on this sequence is "stable": once a value is emitted + // for any piece of incremental state, that value never changes, even + // across SEQ_INCREMENTAL_STATE_CLEARED boundaries. Clears cause + // re-emission of the same values, never new ones. + // + // This means that after packet loss, the trace reader may still process + // these packets because any incremental state that *is* present is + // guaranteed to be correct, even if incomplete. The consumer must be + // prepared to handle missing incremental state lookups. + // + // "Stable" applies to all forms of incremental state including but not + // limited to: + // - Interned data: an iid always maps to the same value. E.g. iid=1 -> + // "malloc" is never re-emitted as iid=1 -> "free" after a clear. + // - TracePacketDefaults: once set, defaults do not change. E.g. + // timestamp_clock_id=6 stays 6 for the entire session. + // - Sequence-scoped clocks: clock snapshots define a clock relationship + // once. After a clear, the same relationship is re-emitted, never a + // new one. + // + // Sequences using this flag MUST NOT use delta encoding for any values + // (e.g. timestamps). Delta encoding is fundamentally incompatible with + // stability because losing a single packet breaks the chain for all + // subsequent values. + SEQ_NEEDS_STABLE_INCREMENTAL_STATE = 4; }; optional uint32 sequence_flags = 13; diff --git a/src/trace_processor/importers/proto/proto_trace_reader.cc b/src/trace_processor/importers/proto/proto_trace_reader.cc index 2f095b8a233..ff61f7b1273 100644 --- a/src/trace_processor/importers/proto/proto_trace_reader.cc +++ b/src/trace_processor/importers/proto/proto_trace_reader.cc @@ -338,17 +338,22 @@ base::Status ProtoTraceReader::ParsePacket(TraceBlobView packet) { } auto* state = GetIncrementalStateForPacketSequence(seq_id); - if (decoder.sequence_flags() & - protos::pbzero::TracePacket::SEQ_NEEDS_INCREMENTAL_STATE) { + bool needs_incremental_state = + sequence_flags & protos::pbzero::TracePacket::SEQ_NEEDS_INCREMENTAL_STATE; + bool needs_stable_incremental_state = + sequence_flags & + protos::pbzero::TracePacket::SEQ_NEEDS_STABLE_INCREMENTAL_STATE; + if (needs_incremental_state || needs_stable_incremental_state) { if (!seq_id) { return base::ErrStatus( - "TracePacket specified SEQ_NEEDS_INCREMENTAL_STATE but the " + "TracePacket specified SEQ_NEEDS_INCREMENTAL_STATE or " + "SEQ_NEEDS_STABLE_INCREMENTAL_STATE but the " "TraceWriter's sequence_id is zero (the service is " "probably too old)"); } scoped_state->needs_incremental_state_total++; - if (!state->IsIncrementalStateValid()) { + if (!state->IsIncrementalStateValid() && !needs_stable_incremental_state) { if (context_->content_analyzer) { // Account for the skipped packet for trace proto content analysis, // with a special annotation.