tp: don't parent thread tracks to process tracks

IMO this was a mistake and never should have been done: the notion of threads nesting under processes is purely a UI construct, doing it at the proto/TP level is a confusion of two concepts. Otherwise, why are processes themselves not nested under the global scope? It also doesn't scale to other "scopes" (e.g. cpus, gpus, linux devices etc) This should have very minimal impact to people's queries: I would argue that relying on this was a bug in the first place, Change-Id: Ib455c1a3f721fb5803cb3ca6c8d22bc175521fd4
google · Dec 23, 2024 · 77f2753 · 77f2753
1 parent b9a8450
commit 77f2753
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 220 deletions.
diff --git a/src/trace_processor/importers/proto/track_event_tracker.cc b/src/trace_processor/importers/proto/track_event_tracker.cc
@@ -174,7 +174,8 @@ std::optional<TrackId> TrackEventTracker::GetDescriptorTrackImpl(
   // We resolve parent_id here to ensure that it's going to be smaller
   // than the id of the child.
   std::optional<TrackId> parent_id;
-  if (reservation.parent_uuid != 0) {
+  if (reservation.parent_uuid != kDefaultDescriptorTrackUuid &&
+      !resolved_track->is_root_in_scope()) {
     parent_id = GetDescriptorTrackImpl(reservation.parent_uuid);
   }
 

diff --git a/test/trace_processor/diff_tests/parser/track_event/tests.py b/test/trace_processor/diff_tests/parser/track_event/tests.py
@@ -300,13 +300,13 @@ def test_track_event_tracks(self):
         "async2","process=p1",1
         "async3","thread=t2",1
         "event_and_track_async3","process=p1",1
-        "process=p1","[NULL]","[NULL]"
+        "process=p1","[NULL]",1
         "process=p2","[NULL]","[NULL]"
         "process=p2","[NULL]","[NULL]"
-        "thread=t1","process=p1",1
-        "thread=t2","process=p1",1
-        "thread=t3","process=p1",1
-        "thread=t4","process=p2","[NULL]"
+        "thread=t1","[NULL]",1
+        "thread=t2","[NULL]",1
+        "thread=t3","[NULL]",1
+        "thread=t4","[NULL]","[NULL]"
         "tid=1","[NULL]","[NULL]"
         """))
 
@@ -853,10 +853,6 @@ def test_track_event_tracks_ordering(self):
         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):
@@ -897,13 +893,13 @@ def test_track_event_tracks_machine_id(self):
         "async2","process=p1",1
         "async3","thread=t2",1
         "event_and_track_async3","process=p1",1
-        "process=p1","[NULL]","[NULL]"
+        "process=p1","[NULL]",1
         "process=p2","[NULL]","[NULL]"
         "process=p2","[NULL]","[NULL]"
-        "thread=t1","process=p1",1
-        "thread=t2","process=p1",1
-        "thread=t3","process=p1",1
-        "thread=t4","process=p2","[NULL]"
+        "thread=t1","[NULL]",1
+        "thread=t2","[NULL]",1
+        "thread=t3","[NULL]",1
+        "thread=t4","[NULL]","[NULL]"
         "tid=1","[NULL]","[NULL]"
         """))
 

diff --git a/test/trace_processor/diff_tests/parser/track_event/track_event_tracks.textproto b/test/trace_processor/diff_tests/parser/track_event/track_event_tracks.textproto
@@ -181,7 +181,6 @@ packet {
   first_packet_on_sequence: true
   track_descriptor {
     uuid: 3
-    parent_uuid: 10
     thread {
       pid: 5
       tid: 1
@@ -234,7 +233,6 @@ packet {
   incremental_state_cleared: true
   track_descriptor {
     uuid: 21
-    parent_uuid: 20
     thread {
       pid: 5
       tid: 4

diff --git a/test/trace_processor/diff_tests/parser/track_event/track_event_tracks_ordering.textproto b/test/trace_processor/diff_tests/parser/track_event/track_event_tracks_ordering.textproto
@@ -7,11 +7,6 @@ packet {
   track_descriptor {
     uuid: 1
     parent_uuid: 10
-    thread {
-      pid: 5
-      tid: 1
-      thread_name: "t1"
-    }
     sibling_order_rank: -10
     disallow_merging_with_system_tracks: true
   }
@@ -30,13 +25,7 @@ packet {
   track_descriptor {
     uuid: 2
     parent_uuid: 10
-    thread {
-      pid: 5
-      tid: 2
-      thread_name: "t2"
-    }
     sibling_order_rank: -2
-    disallow_merging_with_system_tracks: true
   }
   trace_packet_defaults {
     track_event_defaults {
@@ -101,82 +90,6 @@ packet {
     name: "async3"
   }
 }
-
-# Should appear on default track "t1".
-packet {
-  trusted_packet_sequence_id: 1
-  timestamp: 1000
-  track_event {
-    categories: "cat"
-    name: "event1_on_t1"
-    type: 3
-  }
-}
-# Should appear on default track "t2".
-packet {
-  trusted_packet_sequence_id: 2
-  timestamp: 2000
-  track_event {
-    categories: "cat"
-    name: "event1_on_t2"
-    type: 3
-  }
-}
-# Should appear on overridden track "t2".
-packet {
-  trusted_packet_sequence_id: 1
-  timestamp: 3000
-  track_event {
-    track_uuid: 2
-    categories: "cat"
-    name: "event2_on_t2"
-    type: 3
-  }
-}
-# Should appear on process track.
-packet {
-  trusted_packet_sequence_id: 1
-  timestamp: 4000
-  track_event {
-    track_uuid: 10
-    categories: "cat"
-    name: "event1_on_p1"
-    type: 3
-  }
-}
-# Should appear on async track.
-packet {
-  trusted_packet_sequence_id: 1
-  timestamp: 5000
-  track_event {
-    track_uuid: 11
-    categories: "cat"
-    name: "event1_on_async"
-    type: 3
-  }
-}
-# Event for the "async2" track starting on one thread and ending on another.
-packet {
-  trusted_packet_sequence_id: 1
-  timestamp: 5100
-  track_event {
-    track_uuid: 12
-    categories: "cat"
-    name: "event1_on_async2"
-    type: 1
-  }
-}
-packet {
-  trusted_packet_sequence_id: 2
-  timestamp: 5200
-  track_event {
-    track_uuid: 12
-    categories: "cat"
-    name: "event1_on_async2"
-    type: 2
-  }
-}
-
 # If we later see another track descriptor for tid 1, but with a different uuid,
 # we should detect tid reuse and start a new thread.
 packet {
@@ -187,12 +100,6 @@ packet {
   track_descriptor {
     uuid: 3
     parent_uuid: 10
-    thread {
-      pid: 5
-      tid: 1
-      thread_name: "t3"
-    }
-    disallow_merging_with_system_tracks: true
   }
 }
 # Should appear on t3.
@@ -206,7 +113,6 @@ packet {
     type: 3
   }
 }
-
 # If we later see another track descriptor for pid 5, but with a different uuid,
 # we should detect pid reuse and start a new process.
 packet {
@@ -221,121 +127,12 @@ packet {
     }
   }
 }
-# Should appear on p2.
-packet {
-  trusted_packet_sequence_id: 4
-  timestamp: 21000
-  track_event {
-    track_uuid: 20
-    categories: "cat"
-    name: "event1_on_p2"
-    type: 3
-  }
-}
-# Another thread t4 in the new process.
 packet {
   trusted_packet_sequence_id: 4
   timestamp: 22000
   incremental_state_cleared: true
   track_descriptor {
     uuid: 21
     parent_uuid: 20
-    thread {
-      pid: 5
-      tid: 4
-      thread_name: "t4"
-    }
-  }
-}
-# Should appear on t4.
-packet {
-  trusted_packet_sequence_id: 4
-  timestamp: 22000
-  track_event {
-    track_uuid: 21
-    categories: "cat"
-    name: "event1_on_t4"
-    type: 3
-  }
-}
-
-# Another packet for a thread track in the old process, badly sorted.
-packet {
-  trusted_packet_sequence_id: 2
-  timestamp: 6000
-  track_event {
-    track_uuid: 1
-    categories: "cat"
-    name: "event3_on_t1"
-    type: 3
-  }
-}
-
-# Override the track to the default descriptor track for an event with a
-# TrackEvent type. Should appear on the default descriptor track instead of
-# "t1".
-packet {
-  trusted_packet_sequence_id: 1
-  timestamp: 30000
-  track_event {
-    track_uuid: 0
-    categories: "cat"
-    name: "event1_on_t1"
-    type: 3
-  }
-}
-
-# But a legacy event without TrackEvent type falls back to legacy tracks (based
-# on ThreadDescriptor / async IDs / legacy instant scopes). This instant event
-# should appear on the process track "p2".
-packet {
-  trusted_packet_sequence_id: 1
-  timestamp: 31000
-  track_event {
-    track_uuid: 0
-    categories: "cat"
-    name: "event2_on_p2"
-    legacy_event {
-      phase: 73               # 'I'
-      instant_event_scope: 2  # Process scope
-    }
-  }
-}
-
-# And pid/tid overrides take effect even for TrackEvent type events.
-packet {
-  trusted_packet_sequence_id: 1
-  timestamp: 32000
-  track_event {
-    track_uuid: 0
-    categories: "cat"
-    name: "event2_on_t4"
-    type: 3
-    legacy_event {
-      pid_override: 5
-      tid_override: 4
-    }
-  }
-}
-
-# Track descriptor without name and process/thread association derives its
-# name from the first event on the track.
-packet {
-  trusted_packet_sequence_id: 1
-  timestamp: 40000
-  track_descriptor {
-    uuid: 13
-    parent_uuid: 10
-  }
-}
-
-packet {
-  trusted_packet_sequence_id: 1
-  timestamp: 40000
-  track_event {
-    track_uuid: 13
-    categories: "cat"
-    name: "event_and_track_async3"
-    type: 3
   }
 }