Skip to content

[DNS] perfetto: add a new flag for "stable incremental state" #5288

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
LalitMaganti wants to merge 1 commit into
base: main
Choose a base branch
from
+65 −4
Draft

[DNS] perfetto: add a new flag for "stable incremental state" #5288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions protos/perfetto/trace/perfetto_trace.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
28 changes: 28 additions & 0 deletions protos/perfetto/trace/trace_packet.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
13 changes: 9 additions & 4 deletions src/trace_processor/importers/proto/proto_trace_reader.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading