tp: Add child tracks ordering to TrackDescriptor proto

Bug: 367676351
Change-Id: I0bf270a93f2429967c3414319bb6e43d7d5464f7

protos/perfetto/trace/perfetto_trace.proto

@@ -15364,7 +15364,7 @@ message CounterDescriptor {
 // |TrackEvent::track_uuid|. It is possible but not necessary to emit a
 // TrackDescriptor for this implicit track.
 //
-// Next id: 11.
+// Next id: 13.
 message TrackDescriptor {
   // Unique ID that identifies this track. This ID is global to the whole trace.
   // Producers should ensure that it is unlikely to clash with IDs emitted by
@@ -15423,6 +15423,33 @@ message TrackDescriptor {
   // system events use nanoseconds. It results in broken event nesting when
   // track events and system events share a track.
   optional bool disallow_merging_with_system_tracks = 9;
+
+  // How the UI should organise the nested (child) tracks, i.e. tracks where
+  // `parent_uuid` is specified to this track `uuid`.
+  enum ChildTracksOrdering {
+    // The default ordering, with no bearing on how the UI will visualise the
+    // tracks.
+    UNKNOWN = 0;
+
+    // Order tracks by `name` or `static_name` depending on which one has been
+    // specified.
+    LEXICOGRAPHIC = 1;
+
+    // Order tracks by the first `ts` event in a track.
+    CHRONOLOGICAL = 2;
+
+    // Order tracks by `sibling_order_z_index` of children tracks. The
+    // lowest value will be shown before highest values, with tracks without
+    // `sibling_order_z_index` being shown at the end.
+    EXPLICIT = 3;
+  }
+
+  // If set the ordering will be accessible to the UI via the `track` table.
+  optional ChildTracksOrdering child_ordering = 11;
+
+  // Only makes sense if the parent tracks has `child_ordering` set to
+  // `EXPLICIT`. Lowest value has the highest priority.
+  optional int32 sibling_order_z_index = 12;
 }
 
 // End of protos/perfetto/trace/track_event/track_descriptor.proto

protos/perfetto/trace/track_event/track_descriptor.proto

@@ -37,7 +37,7 @@ package perfetto.protos;
 // |TrackEvent::track_uuid|. It is possible but not necessary to emit a
 // TrackDescriptor for this implicit track.
 //
-// Next id: 11.
+// Next id: 13.
 message TrackDescriptor {
   // Unique ID that identifies this track. This ID is global to the whole trace.
   // Producers should ensure that it is unlikely to clash with IDs emitted by
@@ -96,4 +96,31 @@ message TrackDescriptor {
   // system events use nanoseconds. It results in broken event nesting when
   // track events and system events share a track.
   optional bool disallow_merging_with_system_tracks = 9;
+
+  // How the UI should organise the nested (child) tracks, i.e. tracks where
+  // `parent_uuid` is specified to this track `uuid`.
+  enum ChildTracksOrdering {
+    // The default ordering, with no bearing on how the UI will visualise the
+    // tracks.
+    UNKNOWN = 0;
+
+    // Order tracks by `name` or `static_name` depending on which one has been
+    // specified.
+    LEXICOGRAPHIC = 1;
+
+    // Order tracks by the first `ts` event in a track.
+    CHRONOLOGICAL = 2;
+
+    // Order tracks by `sibling_order_z_index` of children tracks. The
+    // lowest value will be shown before highest values, with tracks without
+    // `sibling_order_z_index` being shown at the end.
+    EXPLICIT = 3;
+  }
+
+  // If set the ordering will be accessible to the UI via the `track` table.
+  optional ChildTracksOrdering child_ordering = 11;
+
+  // Only makes sense if the parent tracks has `child_ordering` set to
+  // `EXPLICIT`. Lowest value has the highest priority.
+  optional int32 sibling_order_z_index = 12;
 }

src/trace_processor/importers/proto/track_event_tokenizer.cc

@@ -96,21 +96,51 @@ ModuleResult TrackEventTokenizer::TokenizeTrackDescriptorPacket(
     RefPtr<PacketSequenceStateGeneration> state,
     const protos::pbzero::TracePacket::Decoder& packet,
     int64_t packet_timestamp) {
+  using TrackDescriptorProto = protos::pbzero::TrackDescriptor;
+  using Reservation = TrackEventTracker::DescriptorTrackReservation;
   auto track_descriptor_field = packet.track_descriptor();
-  protos::pbzero::TrackDescriptor::Decoder track(track_descriptor_field.data,
-                                                 track_descriptor_field.size);
+  TrackDescriptorProto::Decoder track(track_descriptor_field.data,
+                                      track_descriptor_field.size);
+
+  Reservation reservation;
 
   if (!track.has_uuid()) {
     PERFETTO_ELOG("TrackDescriptor packet without uuid");
     context_->storage->IncrementStats(stats::track_event_tokenizer_errors);
     return ModuleResult::Handled();
   }
 
-  StringId name_id = kNullStringId;
+  if (track.has_parent_uuid()) {
+    reservation.parent_uuid = track.parent_uuid();
+  }
+
+  if (track.has_child_ordering()) {
+    switch (track.child_ordering()) {
+      case TrackDescriptorProto::ChildTracksOrdering::UNKNOWN:
+        reservation.ordering = Reservation::ChildTracksOrdering::kUnknown;
+        break;
+      case TrackDescriptorProto::ChildTracksOrdering::CHRONOLOGICAL:
+        reservation.ordering = Reservation::ChildTracksOrdering::kChronological;
+        break;
+      case TrackDescriptorProto::ChildTracksOrdering::LEXICOGRAPHIC:
+        reservation.ordering = Reservation::ChildTracksOrdering::kLexicographic;
+        break;
+      case TrackDescriptorProto::ChildTracksOrdering::EXPLICIT:
+        reservation.ordering = Reservation::ChildTracksOrdering::kExplicit;
+        break;
+      default:
+        PERFETTO_FATAL("Unsupported ChildTracksOrdering");
+    }
+  }
+
+  if (track.has_sibling_order_z_index()) {
+    reservation.sibling_order_z_index = track.sibling_order_z_index();
+  }
+
   if (track.has_name())
-    name_id = context_->storage->InternString(track.name());
+    reservation.name = context_->storage->InternString(track.name());
   else if (track.has_static_name())
-    name_id = context_->storage->InternString(track.static_name());
+    reservation.name = context_->storage->InternString(track.static_name());
 
   if (packet.has_trusted_pid()) {
     context_->process_tracker->UpdateTrustedPid(
@@ -132,18 +162,18 @@ ModuleResult TrackEventTokenizer::TokenizeTrackDescriptorPacket(
       TokenizeThreadDescriptor(*state, thread);
     }
 
-    TrackEventTracker::DescriptorTrackReservation reservation;
     reservation.min_timestamp = packet_timestamp;
-    reservation.parent_uuid = track.parent_uuid();
     reservation.pid = static_cast<uint32_t>(thread.pid());
     reservation.tid = static_cast<uint32_t>(thread.tid());
-    reservation.name = name_id;
     reservation.use_separate_track =
         track.disallow_merging_with_system_tracks();
 
     track_event_tracker_->ReserveDescriptorTrack(track.uuid(), reservation);
 
-  } else if (track.has_process()) {
+    return ModuleResult::Ignored();
+  }
+
+  if (track.has_process()) {
     protos::pbzero::ProcessDescriptor::Decoder process(track.process());
 
     if (!process.has_pid()) {
@@ -153,13 +183,13 @@ ModuleResult TrackEventTokenizer::TokenizeTrackDescriptorPacket(
       return ModuleResult::Handled();
     }
 
-    TrackEventTracker::DescriptorTrackReservation reservation;
-    reservation.name = name_id;
     reservation.pid = static_cast<uint32_t>(process.pid());
     reservation.min_timestamp = packet_timestamp;
     track_event_tracker_->ReserveDescriptorTrack(track.uuid(), reservation);
 
-  } else if (track.has_counter()) {
+    return ModuleResult::Ignored();
+  }
+  if (track.has_counter()) {
     protos::pbzero::CounterDescriptor::Decoder counter(track.counter());
 
     StringId category_id = kNullStringId;
@@ -183,22 +213,19 @@ ModuleResult TrackEventTokenizer::TokenizeTrackDescriptorPacket(
     // threads, in which case it has to use absolute values on a different
     // track_uuid. Right now these absolute values are imported onto a separate
     // counter track than the other thread's regular thread time values.)
-    if (name_id.is_null()) {
+    if (reservation.name.is_null()) {
       switch (counter.type()) {
         case CounterDescriptor::COUNTER_UNSPECIFIED:
           break;
         case CounterDescriptor::COUNTER_THREAD_TIME_NS:
-          name_id = counter_name_thread_time_id_;
+          reservation.name = counter_name_thread_time_id_;
           break;
         case CounterDescriptor::COUNTER_THREAD_INSTRUCTION_COUNT:
-          name_id = counter_name_thread_instruction_count_id_;
+          reservation.name = counter_name_thread_instruction_count_id_;
           break;
       }
     }
 
-    TrackEventTracker::DescriptorTrackReservation reservation;
-    reservation.parent_uuid = track.parent_uuid();
-    reservation.name = name_id;
     reservation.is_counter = true;
 
     TrackEventTracker::DescriptorTrackReservation::CounterDetails
@@ -214,16 +241,14 @@ ModuleResult TrackEventTokenizer::TokenizeTrackDescriptorPacket(
     reservation.counter_details = std::move(counter_details);
     track_event_tracker_->ReserveDescriptorTrack(track.uuid(), reservation);
 
-  } else {
-    TrackEventTracker::DescriptorTrackReservation r;
-    r.name = name_id;
-    r.parent_uuid = track.parent_uuid();
-    track_event_tracker_->ReserveDescriptorTrack(track.uuid(), r);
+    return ModuleResult::Ignored();
   }
 
+  track_event_tracker_->ReserveDescriptorTrack(track.uuid(), reservation);
+
   // Let ProtoTraceReader forward the packet to the parser.
   return ModuleResult::Ignored();
-}
+}  // namespace perfetto::trace_processor
 
 ModuleResult TrackEventTokenizer::TokenizeThreadDescriptorPacket(
     RefPtr<PacketSequenceStateGeneration> state,

src/trace_processor/importers/proto/track_event_tracker.cc

@@ -47,6 +47,12 @@ TrackEventTracker::TrackEventTracker(TraceProcessorContext* context)
       category_key_(context->storage->InternString("category")),
       has_first_packet_on_sequence_key_id_(
           context->storage->InternString("has_first_packet_on_sequence")),
+      child_ordering_key_(context->storage->InternString("child_ordering")),
+      explicit_id_(context->storage->InternString("explicit")),
+      lexicographic_id_(context->storage->InternString("lexicographic")),
+      chronological_id_(context->storage->InternString("chronological")),
+      sibling_order_z_index_key_(
+          context->storage->InternString("sibling_order_z_index")),
       descriptor_source_(context->storage->InternString("descriptor")),
       default_descriptor_track_name_(
           context->storage->InternString("Default Track")),
@@ -147,6 +153,25 @@ std::optional<TrackId> TrackEventTracker::GetDescriptorTrackImpl(
     args.AddArg(has_first_packet_on_sequence_key_id_, Variadic::Boolean(true));
   }
 
+  switch (reservation.ordering) {
+    case DescriptorTrackReservation::ChildTracksOrdering::kLexicographic:
+      args.AddArg(child_ordering_key_, Variadic::String(lexicographic_id_));
+      break;
+    case DescriptorTrackReservation::ChildTracksOrdering::kChronological:
+      args.AddArg(child_ordering_key_, Variadic::String(chronological_id_));
+      break;
+    case DescriptorTrackReservation::ChildTracksOrdering::kExplicit:
+      args.AddArg(child_ordering_key_, Variadic::String(explicit_id_));
+      break;
+    case DescriptorTrackReservation::ChildTracksOrdering::kUnknown:
+      break;
+  }
+
+  if (reservation.sibling_order_z_index) {
+    args.AddArg(sibling_order_z_index_key_,
+                Variadic::Integer(*reservation.sibling_order_z_index));
+  }
+
   auto row_ref = *context_->storage->mutable_track_table()->FindById(track_id);
   if (parent_id) {
     row_ref.set_parent_id(*parent_id);

src/trace_processor/importers/proto/track_event_tracker.h

@@ -39,8 +39,15 @@ class TrackEventTracker {
   // Data from TrackDescriptor proto used to reserve a track before interning it
   // with |TrackTracker|.
   struct DescriptorTrackReservation {
+    // Maps to TrackDescriptor::ChildTracksOrdering proto values
+    enum class ChildTracksOrdering {
+      kUnknown = 0,
+      kLexicographic = 1,
+      kChronological = 2,
+      kExplicit = 3,
+    };
     struct CounterDetails {
-      StringId category = kNullStringId;
+      StringId category;
       int64_t unit_multiplier = 1;
       bool is_incremental = false;
       uint32_t packet_sequence_id = 0;
@@ -65,6 +72,10 @@ class TrackEventTracker {
     // For counter tracks.
     std::optional<CounterDetails> counter_details;
 
+    // For UI visualisation
+    ChildTracksOrdering ordering = ChildTracksOrdering::kUnknown;
+    std::optional<int32_t> sibling_order_z_index;
+
     // Whether |other| is a valid descriptor for this track reservation. A track
     // should always remain nested underneath its original parent.
     bool IsForSameTrack(const DescriptorTrackReservation& other) {
@@ -205,15 +216,20 @@ class TrackEventTracker {
 
   std::unordered_set<uint32_t> sequences_with_first_packet_;
 
-  const StringId source_key_ = kNullStringId;
-  const StringId source_id_key_ = kNullStringId;
-  const StringId is_root_in_scope_key_ = kNullStringId;
-  const StringId category_key_ = kNullStringId;
-  const StringId has_first_packet_on_sequence_key_id_ = kNullStringId;
+  const StringId source_key_;
+  const StringId source_id_key_;
+  const StringId is_root_in_scope_key_;
+  const StringId category_key_;
+  const StringId has_first_packet_on_sequence_key_id_;
+  const StringId child_ordering_key_;
+  const StringId explicit_id_;
+  const StringId lexicographic_id_;
+  const StringId chronological_id_;
+  const StringId sibling_order_z_index_key_;
 
-  const StringId descriptor_source_ = kNullStringId;
+  const StringId descriptor_source_;
 
-  const StringId default_descriptor_track_name_ = kNullStringId;
+  const StringId default_descriptor_track_name_;
 
   std::optional<int64_t> range_of_interest_start_us_;
 

test/trace_processor/diff_tests/parser/track_event/tests.py

@@ -805,6 +805,34 @@ def test_range_of_interest(self):
         13000,"slice4"
         """))
 
+  def test_track_event_tracks_ordering(self):
+    return DiffTestBlueprint(
+        trace=Path('track_event_tracks_ordering.textproto'),
+        query="""
+        SELECT
+          id,
+          parent_id,
+          EXTRACT_ARG(source_arg_set_id, 'child_ordering') AS ordering,
+          EXTRACT_ARG(source_arg_set_id, 'sibling_order_z_index') AS z_index
+        FROM track
+        """,
+        out=Csv("""
+        "id","parent_id","ordering","z_index"
+        0,"[NULL]","explicit","[NULL]"
+        1,0,"[NULL]",-10
+        2,0,"[NULL]",-2
+        3,0,"[NULL]",1
+        4,0,"[NULL]",2
+        5,2,"[NULL]","[NULL]"
+        6,0,"[NULL]","[NULL]"
+        7,"[NULL]","[NULL]","[NULL]"
+        8,7,"[NULL]","[NULL]"
+        9,"[NULL]","[NULL]","[NULL]"
+        10,"[NULL]","[NULL]","[NULL]"
+        11,"[NULL]","[NULL]","[NULL]"
+        12,0,"[NULL]","[NULL]"
+        """))
+
   def test_track_event_tracks_machine_id(self):
     return DiffTestBlueprint(
         trace=Path('track_event_tracks.textproto'),