processor: Parse StreamingProfilePacket timestamps during tokenization

Oystein discovered that resolving delta-timestamps during the parsing
stage is problematic, because a later ThreadDescriptor may reset the
reference value before its preceeding packets are parsed.

Simplest fix seems to be to move resolution of StreamingProfilePacket
reference timestamps into the tokenization phase. See also discussion
on aosp/1234246. Resolution of pid/tids has to remain in the parsing
phase, so we only insert into tables then.

This patch moves StreamingProfilePacket parsing into a new
ProfileModule to accomplish this. It also makes a few minor changes to
the way in which we register modules, to simplify test setup.

Bug: chromium:1046918
Change-Id: I34868e0ba9365bc050a87b654975f3089d33f341
diff --git a/Android.bp b/Android.bp
index 5c70f53..af1f5d6 100644
--- a/Android.bp
+++ b/Android.bp
@@ -6256,6 +6256,7 @@
 filegroup {
   name: "perfetto_src_trace_processor_storage_full",
   srcs: [
+    "src/trace_processor/additional_modules.cc",
     "src/trace_processor/importers/ftrace/binder_tracker.cc",
     "src/trace_processor/importers/ftrace/ftrace_descriptors.cc",
     "src/trace_processor/importers/ftrace/ftrace_module_impl.cc",
@@ -6275,7 +6276,6 @@
     "src/trace_processor/importers/proto/vulkan_memory_tracker.cc",
     "src/trace_processor/importers/systrace/systrace_parser.cc",
     "src/trace_processor/importers/systrace/systrace_trace_parser.cc",
-    "src/trace_processor/register_additional_modules.cc",
     "src/trace_processor/syscall_tracker.cc",
   ],
 }
@@ -6286,6 +6286,7 @@
   srcs: [
     "src/trace_processor/args_tracker.cc",
     "src/trace_processor/clock_tracker.cc",
+    "src/trace_processor/default_modules.cc",
     "src/trace_processor/destructible.cc",
     "src/trace_processor/event_tracker.cc",
     "src/trace_processor/forwarding_trace_parser.cc",
@@ -6300,6 +6301,8 @@
     "src/trace_processor/importers/fuchsia/fuchsia_trace_utils.cc",
     "src/trace_processor/importers/proto/args_table_utils.cc",
     "src/trace_processor/importers/proto/packet_sequence_state.cc",
+    "src/trace_processor/importers/proto/profile_module.cc",
+    "src/trace_processor/importers/proto/profile_packet_utils.cc",
     "src/trace_processor/importers/proto/proto_importer_module.cc",
     "src/trace_processor/importers/proto/proto_trace_parser.cc",
     "src/trace_processor/importers/proto/proto_trace_tokenizer.cc",
diff --git a/BUILD b/BUILD
index bce6463..6b659c9 100644
--- a/BUILD
+++ b/BUILD
@@ -866,6 +866,8 @@
 filegroup(
     name = "src_trace_processor_storage_full",
     srcs = [
+        "src/trace_processor/additional_modules.cc",
+        "src/trace_processor/additional_modules.h",
         "src/trace_processor/importers/ftrace/binder_tracker.cc",
         "src/trace_processor/importers/ftrace/binder_tracker.h",
         "src/trace_processor/importers/ftrace/ftrace_descriptors.cc",
@@ -904,8 +906,6 @@
         "src/trace_processor/importers/systrace/systrace_parser.h",
         "src/trace_processor/importers/systrace/systrace_trace_parser.cc",
         "src/trace_processor/importers/systrace/systrace_trace_parser.h",
-        "src/trace_processor/register_additional_modules.cc",
-        "src/trace_processor/register_additional_modules.h",
         "src/trace_processor/syscall_tracker.cc",
         "src/trace_processor/syscalls_aarch32.h",
         "src/trace_processor/syscalls_aarch64.h",
@@ -923,6 +923,8 @@
         "src/trace_processor/chunked_trace_reader.h",
         "src/trace_processor/clock_tracker.cc",
         "src/trace_processor/clock_tracker.h",
+        "src/trace_processor/default_modules.cc",
+        "src/trace_processor/default_modules.h",
         "src/trace_processor/destructible.cc",
         "src/trace_processor/destructible.h",
         "src/trace_processor/event_tracker.cc",
@@ -960,6 +962,10 @@
         "src/trace_processor/importers/proto/chrome_compositor_scheduler_state.descriptor.h",
         "src/trace_processor/importers/proto/packet_sequence_state.cc",
         "src/trace_processor/importers/proto/packet_sequence_state.h",
+        "src/trace_processor/importers/proto/profile_module.cc",
+        "src/trace_processor/importers/proto/profile_module.h",
+        "src/trace_processor/importers/proto/profile_packet_utils.cc",
+        "src/trace_processor/importers/proto/profile_packet_utils.h",
         "src/trace_processor/importers/proto/proto_importer_module.cc",
         "src/trace_processor/importers/proto/proto_importer_module.h",
         "src/trace_processor/importers/proto/proto_incremental_state.h",
diff --git a/src/trace_processor/BUILD.gn b/src/trace_processor/BUILD.gn
index 3e70300..cbe1d9b 100644
--- a/src/trace_processor/BUILD.gn
+++ b/src/trace_processor/BUILD.gn
@@ -79,6 +79,8 @@
     "chunked_trace_reader.h",
     "clock_tracker.cc",
     "clock_tracker.h",
+    "default_modules.cc",
+    "default_modules.h",
     "destructible.cc",
     "destructible.h",
     "event_tracker.cc",
@@ -98,6 +100,10 @@
     "importers/proto/chrome_compositor_scheduler_state.descriptor.h",
     "importers/proto/packet_sequence_state.cc",
     "importers/proto/packet_sequence_state.h",
+    "importers/proto/profile_module.cc",
+    "importers/proto/profile_module.h",
+    "importers/proto/profile_packet_utils.cc",
+    "importers/proto/profile_packet_utils.h",
     "importers/proto/proto_importer_module.cc",
     "importers/proto/proto_importer_module.h",
     "importers/proto/proto_incremental_state.h",
@@ -200,6 +206,8 @@
 
 source_set("storage_full") {
   sources = [
+    "additional_modules.cc",
+    "additional_modules.h",
     "importers/ftrace/binder_tracker.cc",
     "importers/ftrace/binder_tracker.h",
     "importers/ftrace/ftrace_descriptors.cc",
@@ -238,8 +246,6 @@
     "importers/systrace/systrace_parser.h",
     "importers/systrace/systrace_trace_parser.cc",
     "importers/systrace/systrace_trace_parser.h",
-    "register_additional_modules.cc",
-    "register_additional_modules.h",
     "syscall_tracker.cc",
     "syscalls_aarch32.h",
     "syscalls_aarch64.h",
diff --git a/src/trace_processor/register_additional_modules.cc b/src/trace_processor/additional_modules.cc
similarity index 96%
rename from src/trace_processor/register_additional_modules.cc
rename to src/trace_processor/additional_modules.cc
index ad14f74..3b992b3 100644
--- a/src/trace_processor/register_additional_modules.cc
+++ b/src/trace_processor/additional_modules.cc
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-#include "src/trace_processor/register_additional_modules.h"
+#include "src/trace_processor/additional_modules.h"
 #include "src/trace_processor/importers/ftrace/ftrace_module_impl.h"
 #include "src/trace_processor/importers/proto/android_probes_module.h"
 #include "src/trace_processor/importers/proto/graphics_event_module.h"
diff --git a/src/trace_processor/register_additional_modules.h b/src/trace_processor/additional_modules.h
similarity index 82%
rename from src/trace_processor/register_additional_modules.h
rename to src/trace_processor/additional_modules.h
index 3b888d5..d68720d 100644
--- a/src/trace_processor/register_additional_modules.h
+++ b/src/trace_processor/additional_modules.h
@@ -14,8 +14,8 @@
  * limitations under the License.
  */
 
-#ifndef SRC_TRACE_PROCESSOR_REGISTER_ADDITIONAL_MODULES_H_
-#define SRC_TRACE_PROCESSOR_REGISTER_ADDITIONAL_MODULES_H_
+#ifndef SRC_TRACE_PROCESSOR_ADDITIONAL_MODULES_H_
+#define SRC_TRACE_PROCESSOR_ADDITIONAL_MODULES_H_
 
 #include "src/trace_processor/trace_processor_context.h"
 
@@ -27,4 +27,4 @@
 }  // namespace trace_processor
 }  // namespace perfetto
 
-#endif  // SRC_TRACE_PROCESSOR_REGISTER_ADDITIONAL_MODULES_H_
+#endif  // SRC_TRACE_PROCESSOR_ADDITIONAL_MODULES_H_
diff --git a/src/trace_processor/default_modules.cc b/src/trace_processor/default_modules.cc
new file mode 100644
index 0000000..3a28c7a
--- /dev/null
+++ b/src/trace_processor/default_modules.cc
@@ -0,0 +1,38 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "src/trace_processor/default_modules.h"
+#include "src/trace_processor/importers/ftrace/ftrace_module.h"
+#include "src/trace_processor/importers/proto/profile_module.h"
+#include "src/trace_processor/importers/proto/proto_importer_module.h"
+#include "src/trace_processor/importers/proto/track_event_module.h"
+
+namespace perfetto {
+namespace trace_processor {
+
+void RegisterDefaultModules(TraceProcessorContext* context) {
+  context->modules.emplace_back(new FtraceModule());
+  // Ftrace module is special, because it has one extra method for parsing
+  // ftrace packets. So we need to store a pointer to it separately.
+  context->ftrace_module =
+      static_cast<FtraceModule*>(context->modules.back().get());
+
+  context->modules.emplace_back(new TrackEventModule(context));
+  context->modules.emplace_back(new ProfileModule(context));
+}
+
+}  // namespace trace_processor
+}  // namespace perfetto
diff --git a/src/trace_processor/register_additional_modules.h b/src/trace_processor/default_modules.h
similarity index 71%
copy from src/trace_processor/register_additional_modules.h
copy to src/trace_processor/default_modules.h
index 3b888d5..97da4e8 100644
--- a/src/trace_processor/register_additional_modules.h
+++ b/src/trace_processor/default_modules.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2019 The Android Open Source Project
+ * Copyright (C) 2020 The Android Open Source Project
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -14,17 +14,17 @@
  * limitations under the License.
  */
 
-#ifndef SRC_TRACE_PROCESSOR_REGISTER_ADDITIONAL_MODULES_H_
-#define SRC_TRACE_PROCESSOR_REGISTER_ADDITIONAL_MODULES_H_
+#ifndef SRC_TRACE_PROCESSOR_DEFAULT_MODULES_H_
+#define SRC_TRACE_PROCESSOR_DEFAULT_MODULES_H_
 
 #include "src/trace_processor/trace_processor_context.h"
 
 namespace perfetto {
 namespace trace_processor {
 
-void RegisterAdditionalModules(TraceProcessorContext*);
+void RegisterDefaultModules(TraceProcessorContext*);
 
 }  // namespace trace_processor
 }  // namespace perfetto
 
-#endif  // SRC_TRACE_PROCESSOR_REGISTER_ADDITIONAL_MODULES_H_
+#endif  // SRC_TRACE_PROCESSOR_DEFAULT_MODULES_H_
diff --git a/src/trace_processor/importers/proto/profile_module.cc b/src/trace_processor/importers/proto/profile_module.cc
new file mode 100644
index 0000000..a6c49cd
--- /dev/null
+++ b/src/trace_processor/importers/proto/profile_module.cc
@@ -0,0 +1,152 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "src/trace_processor/importers/proto/profile_module.h"
+
+#include "perfetto/base/logging.h"
+#include "src/trace_processor/clock_tracker.h"
+#include "src/trace_processor/importers/proto/packet_sequence_state.h"
+#include "src/trace_processor/importers/proto/profile_packet_utils.h"
+#include "src/trace_processor/process_tracker.h"
+#include "src/trace_processor/stack_profile_tracker.h"
+#include "src/trace_processor/storage/trace_storage.h"
+#include "src/trace_processor/tables/profiler_tables.h"
+#include "src/trace_processor/timestamped_trace_piece.h"
+#include "src/trace_processor/trace_processor_context.h"
+#include "src/trace_processor/trace_sorter.h"
+
+#include "protos/perfetto/trace/clock_snapshot.pbzero.h"
+#include "protos/perfetto/trace/profiling/profile_packet.pbzero.h"
+
+namespace perfetto {
+namespace trace_processor {
+
+using perfetto::protos::pbzero::TracePacket;
+using protozero::ConstBytes;
+
+ProfileModule::ProfileModule(TraceProcessorContext* context)
+    : context_(context) {
+  RegisterForField(TracePacket::kStreamingProfilePacketFieldNumber, context);
+}
+
+ProfileModule::~ProfileModule() = default;
+
+ModuleResult ProfileModule::TokenizePacket(const TracePacket::Decoder& decoder,
+                                           TraceBlobView* packet,
+                                           int64_t /*packet_timestamp*/,
+                                           PacketSequenceState* state,
+                                           uint32_t field_id) {
+  switch (field_id) {
+    case TracePacket::kStreamingProfilePacketFieldNumber:
+      return TokenizeStreamingProfilePacket(state, packet,
+                                            decoder.streaming_profile_packet());
+  }
+  return ModuleResult::Ignored();
+}
+
+void ProfileModule::ParsePacket(const TracePacket::Decoder& decoder,
+                                const TimestampedTracePiece& ttp,
+                                uint32_t field_id) {
+  switch (field_id) {
+    case TracePacket::kStreamingProfilePacketFieldNumber:
+      PERFETTO_DCHECK(ttp.type == TimestampedTracePiece::Type::kTracePacket);
+      ParseStreamingProfilePacket(ttp.timestamp, ttp.packet_data.sequence_state,
+                                  decoder.streaming_profile_packet());
+      return;
+  }
+}
+
+ModuleResult ProfileModule::TokenizeStreamingProfilePacket(
+    PacketSequenceState* sequence_state,
+    TraceBlobView* packet,
+    ConstBytes streaming_profile_packet) {
+  protos::pbzero::StreamingProfilePacket::Decoder decoder(
+      streaming_profile_packet.data, streaming_profile_packet.size);
+
+  // We have to resolve the reference timestamp of a StreamingProfilePacket
+  // during tokenization. If we did this during parsing instead, the
+  // tokenization of a subsequent ThreadDescriptor with a new reference
+  // timestamp would cause us to later calculate timestamps based on the wrong
+  // reference value during parsing. Since StreamingProfilePackets only need to
+  // be sorted correctly with respect to process/thread metadata events (so that
+  // pid/tid are resolved correctly during parsing), we forward the packet as a
+  // whole through the sorter, using the "root" timestamp of the packet, i.e.
+  // the current timestamp of the packet sequence.
+  auto packet_ts =
+      sequence_state->IncrementAndGetTrackEventTimeNs(/*delta_ns=*/0);
+  auto trace_ts = context_->clock_tracker->ToTraceTime(
+      protos::pbzero::ClockSnapshot::Clock::MONOTONIC, packet_ts);
+  if (trace_ts)
+    packet_ts = *trace_ts;
+
+  // Increment the sequence's timestamp by all deltas.
+  for (auto timestamp_it = decoder.timestamp_delta_us(); timestamp_it;
+       ++timestamp_it) {
+    sequence_state->IncrementAndGetTrackEventTimeNs(*timestamp_it * 1000);
+  }
+
+  context_->sorter->PushTracePacket(packet_ts, sequence_state,
+                                    std::move(*packet));
+  return ModuleResult::Handled();
+}
+
+void ProfileModule::ParseStreamingProfilePacket(
+    int64_t timestamp,
+    PacketSequenceStateGeneration* sequence_state,
+    ConstBytes streaming_profile_packet) {
+  protos::pbzero::StreamingProfilePacket::Decoder packet(
+      streaming_profile_packet.data, streaming_profile_packet.size);
+
+  ProcessTracker* procs = context_->process_tracker.get();
+  TraceStorage* storage = context_->storage.get();
+  StackProfileTracker& stack_profile_tracker =
+      sequence_state->state()->stack_profile_tracker();
+  ProfilePacketInternLookup intern_lookup(sequence_state);
+
+  uint32_t pid = static_cast<uint32_t>(sequence_state->state()->pid());
+  uint32_t tid = static_cast<uint32_t>(sequence_state->state()->tid());
+  UniqueTid utid = procs->UpdateThread(tid, pid);
+
+  // Iterate through timestamps and callstacks simultaneously.
+  auto timestamp_it = packet.timestamp_delta_us();
+  for (auto callstack_it = packet.callstack_iid(); callstack_it;
+       ++callstack_it, ++timestamp_it) {
+    if (!timestamp_it) {
+      context_->storage->IncrementStats(stats::stackprofile_parser_error);
+      PERFETTO_ELOG(
+          "StreamingProfilePacket has less callstack IDs than timestamps!");
+      break;
+    }
+
+    auto opt_cs_id = stack_profile_tracker.FindOrInsertCallstack(
+        *callstack_it, &intern_lookup);
+    if (!opt_cs_id) {
+      context_->storage->IncrementStats(stats::stackprofile_parser_error);
+      PERFETTO_ELOG("StreamingProfilePacket referencing invalid callstack!");
+      continue;
+    }
+
+    // Resolve the delta timestamps based on the packet's root timestamp.
+    timestamp += *timestamp_it * 1000;
+
+    tables::CpuProfileStackSampleTable::Row sample_row{timestamp, *opt_cs_id,
+                                                       utid};
+    storage->mutable_cpu_profile_stack_sample_table()->Insert(sample_row);
+  }
+}
+
+}  // namespace trace_processor
+}  // namespace perfetto
diff --git a/src/trace_processor/importers/proto/profile_module.h b/src/trace_processor/importers/proto/profile_module.h
new file mode 100644
index 0000000..fe30e03
--- /dev/null
+++ b/src/trace_processor/importers/proto/profile_module.h
@@ -0,0 +1,64 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef SRC_TRACE_PROCESSOR_IMPORTERS_PROTO_PROFILE_MODULE_H_
+#define SRC_TRACE_PROCESSOR_IMPORTERS_PROTO_PROFILE_MODULE_H_
+
+#include "perfetto/protozero/field.h"
+#include "src/trace_processor/importers/proto/proto_importer_module.h"
+#include "src/trace_processor/importers/proto/proto_incremental_state.h"
+
+#include "protos/perfetto/trace/trace_packet.pbzero.h"
+
+namespace perfetto {
+namespace trace_processor {
+
+// Importer module for heap and CPU sampling profile data.
+// TODO(eseckler): Currently handles only StreamingProfilePackets. Also move
+// other profiling data import functionality into this module.
+class ProfileModule : public ProtoImporterModule {
+ public:
+  explicit ProfileModule(TraceProcessorContext* context);
+  ~ProfileModule() override;
+
+  ModuleResult TokenizePacket(
+      const protos::pbzero::TracePacket::Decoder& decoder,
+      TraceBlobView* packet,
+      int64_t packet_timestamp,
+      PacketSequenceState* state,
+      uint32_t field_id) override;
+
+  void ParsePacket(const protos::pbzero::TracePacket::Decoder& decoder,
+                   const TimestampedTracePiece& ttp,
+                   uint32_t field_id) override;
+
+ private:
+  ModuleResult TokenizeStreamingProfilePacket(
+      PacketSequenceState*,
+      TraceBlobView* packet,
+      protozero::ConstBytes streaming_profile_packet);
+  void ParseStreamingProfilePacket(
+      int64_t timestamp,
+      PacketSequenceStateGeneration*,
+      protozero::ConstBytes streaming_profile_packet);
+
+  TraceProcessorContext* context_;
+};
+
+}  // namespace trace_processor
+}  // namespace perfetto
+
+#endif  // SRC_TRACE_PROCESSOR_IMPORTERS_PROTO_PROFILE_MODULE_H_
diff --git a/src/trace_processor/register_additional_modules.h b/src/trace_processor/importers/proto/profile_packet_utils.cc
similarity index 65%
copy from src/trace_processor/register_additional_modules.h
copy to src/trace_processor/importers/proto/profile_packet_utils.cc
index 3b888d5..9478db9 100644
--- a/src/trace_processor/register_additional_modules.h
+++ b/src/trace_processor/importers/proto/profile_packet_utils.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2019 The Android Open Source Project
+ * Copyright (C) 2020 The Android Open Source Project
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -14,17 +14,12 @@
  * limitations under the License.
  */
 
-#ifndef SRC_TRACE_PROCESSOR_REGISTER_ADDITIONAL_MODULES_H_
-#define SRC_TRACE_PROCESSOR_REGISTER_ADDITIONAL_MODULES_H_
-
-#include "src/trace_processor/trace_processor_context.h"
+#include "src/trace_processor/importers/proto/profile_packet_utils.h"
 
 namespace perfetto {
 namespace trace_processor {
 
-void RegisterAdditionalModules(TraceProcessorContext*);
+ProfilePacketInternLookup::~ProfilePacketInternLookup() = default;
 
 }  // namespace trace_processor
 }  // namespace perfetto
-
-#endif  // SRC_TRACE_PROCESSOR_REGISTER_ADDITIONAL_MODULES_H_
diff --git a/src/trace_processor/importers/proto/profile_packet_utils.h b/src/trace_processor/importers/proto/profile_packet_utils.h
new file mode 100644
index 0000000..79c63bf
--- /dev/null
+++ b/src/trace_processor/importers/proto/profile_packet_utils.h
@@ -0,0 +1,137 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef SRC_TRACE_PROCESSOR_IMPORTERS_PROTO_PROFILE_PACKET_UTILS_H_
+#define SRC_TRACE_PROCESSOR_IMPORTERS_PROTO_PROFILE_PACKET_UTILS_H_
+
+#include "perfetto/ext/base/optional.h"
+#include "perfetto/ext/base/string_view.h"
+#include "src/trace_processor/importers/proto/packet_sequence_state.h"
+#include "src/trace_processor/stack_profile_tracker.h"
+
+#include "protos/perfetto/trace/interned_data/interned_data.pbzero.h"
+#include "protos/perfetto/trace/profiling/profile_common.pbzero.h"
+
+namespace perfetto {
+namespace trace_processor {
+
+class ProfilePacketUtils {
+ public:
+  static StackProfileTracker::SourceMapping MakeSourceMapping(
+      const protos::pbzero::Mapping::Decoder& entry) {
+    StackProfileTracker::SourceMapping src_mapping{};
+    src_mapping.build_id = entry.build_id();
+    src_mapping.exact_offset = entry.exact_offset();
+    src_mapping.start_offset = entry.start_offset();
+    src_mapping.start = entry.start();
+    src_mapping.end = entry.end();
+    src_mapping.load_bias = entry.load_bias();
+    for (auto path_string_id_it = entry.path_string_ids(); path_string_id_it;
+         ++path_string_id_it) {
+      src_mapping.name_ids.emplace_back(*path_string_id_it);
+    }
+    return src_mapping;
+  }
+
+  static StackProfileTracker::SourceFrame MakeSourceFrame(
+      const protos::pbzero::Frame::Decoder& entry) {
+    StackProfileTracker::SourceFrame src_frame;
+    src_frame.name_id = entry.function_name_id();
+    src_frame.mapping_id = entry.mapping_id();
+    src_frame.rel_pc = entry.rel_pc();
+    return src_frame;
+  }
+
+  static StackProfileTracker::SourceCallstack MakeSourceCallstack(
+      const protos::pbzero::Callstack::Decoder& entry) {
+    StackProfileTracker::SourceCallstack src_callstack;
+    for (auto frame_it = entry.frame_ids(); frame_it; ++frame_it)
+      src_callstack.emplace_back(*frame_it);
+    return src_callstack;
+  }
+};
+
+class ProfilePacketInternLookup : public StackProfileTracker::InternLookup {
+ public:
+  explicit ProfilePacketInternLookup(PacketSequenceStateGeneration* seq_state)
+      : seq_state_(seq_state) {}
+  ~ProfilePacketInternLookup() override;
+
+  base::Optional<base::StringView> GetString(
+      StackProfileTracker::SourceStringId iid,
+      StackProfileTracker::InternedStringType type) const override {
+    protos::pbzero::InternedString::Decoder* decoder = nullptr;
+    switch (type) {
+      case StackProfileTracker::InternedStringType::kBuildId:
+        decoder = seq_state_->LookupInternedMessage<
+            protos::pbzero::InternedData::kBuildIdsFieldNumber,
+            protos::pbzero::InternedString>(iid);
+        break;
+      case StackProfileTracker::InternedStringType::kFunctionName:
+        decoder = seq_state_->LookupInternedMessage<
+            protos::pbzero::InternedData::kFunctionNamesFieldNumber,
+            protos::pbzero::InternedString>(iid);
+        break;
+      case StackProfileTracker::InternedStringType::kMappingPath:
+        decoder = seq_state_->LookupInternedMessage<
+            protos::pbzero::InternedData::kMappingPathsFieldNumber,
+            protos::pbzero::InternedString>(iid);
+        break;
+    }
+    if (!decoder)
+      return base::nullopt;
+    return base::StringView(reinterpret_cast<const char*>(decoder->str().data),
+                            decoder->str().size);
+  }
+
+  base::Optional<StackProfileTracker::SourceMapping> GetMapping(
+      StackProfileTracker::SourceMappingId iid) const override {
+    auto* decoder = seq_state_->LookupInternedMessage<
+        protos::pbzero::InternedData::kMappingsFieldNumber,
+        protos::pbzero::Mapping>(iid);
+    if (!decoder)
+      return base::nullopt;
+    return ProfilePacketUtils::MakeSourceMapping(*decoder);
+  }
+
+  base::Optional<StackProfileTracker::SourceFrame> GetFrame(
+      StackProfileTracker::SourceFrameId iid) const override {
+    auto* decoder = seq_state_->LookupInternedMessage<
+        protos::pbzero::InternedData::kFramesFieldNumber,
+        protos::pbzero::Frame>(iid);
+    if (!decoder)
+      return base::nullopt;
+    return ProfilePacketUtils::MakeSourceFrame(*decoder);
+  }
+
+  base::Optional<StackProfileTracker::SourceCallstack> GetCallstack(
+      StackProfileTracker::SourceCallstackId iid) const override {
+    auto* decoder = seq_state_->LookupInternedMessage<
+        protos::pbzero::InternedData::kCallstacksFieldNumber,
+        protos::pbzero::Callstack>(iid);
+    if (!decoder)
+      return base::nullopt;
+    return ProfilePacketUtils::MakeSourceCallstack(*decoder);
+  }
+
+ private:
+  PacketSequenceStateGeneration* seq_state_;
+};
+
+}  // namespace trace_processor
+}  // namespace perfetto
+
+#endif  // SRC_TRACE_PROCESSOR_IMPORTERS_PROTO_PROFILE_PACKET_UTILS_H_
diff --git a/src/trace_processor/importers/proto/proto_importer_module.h b/src/trace_processor/importers/proto/proto_importer_module.h
index 60a0218..97e286b 100644
--- a/src/trace_processor/importers/proto/proto_importer_module.h
+++ b/src/trace_processor/importers/proto/proto_importer_module.h
@@ -45,8 +45,8 @@
 //     methods.
 // (2) In the constructor call the RegisterForField method for every field
 //     that the module knows how to handle.
-// (3) Create a module instance and add it to the |modules| vector in
-//     TraceProcessorContext.
+// (3) Create a module instance and add it to TraceProcessorContext's |modules|
+//     vector in either default_modules.cc or additional_modules.cc.
 // See GraphicsEventModule for an example.
 
 class ModuleResult {
diff --git a/src/trace_processor/importers/proto/proto_trace_parser.cc b/src/trace_processor/importers/proto/proto_trace_parser.cc
index 23d977f..d3c585b 100644
--- a/src/trace_processor/importers/proto/proto_trace_parser.cc
+++ b/src/trace_processor/importers/proto/proto_trace_parser.cc
@@ -35,6 +35,7 @@
 #include "src/trace_processor/heap_profile_tracker.h"
 #include "src/trace_processor/importers/ftrace/ftrace_module.h"
 #include "src/trace_processor/importers/proto/packet_sequence_state.h"
+#include "src/trace_processor/importers/proto/profile_packet_utils.h"
 #include "src/trace_processor/metadata_tracker.h"
 #include "src/trace_processor/perf_sample_tracker.h"
 #include "src/trace_processor/process_tracker.h"
@@ -61,108 +62,6 @@
 namespace perfetto {
 namespace trace_processor {
 
-namespace {
-
-StackProfileTracker::SourceMapping MakeSourceMapping(
-    const protos::pbzero::Mapping::Decoder& entry) {
-  StackProfileTracker::SourceMapping src_mapping{};
-  src_mapping.build_id = entry.build_id();
-  src_mapping.exact_offset = entry.exact_offset();
-  src_mapping.start_offset = entry.start_offset();
-  src_mapping.start = entry.start();
-  src_mapping.end = entry.end();
-  src_mapping.load_bias = entry.load_bias();
-  for (auto path_string_id_it = entry.path_string_ids(); path_string_id_it;
-       ++path_string_id_it)
-    src_mapping.name_ids.emplace_back(*path_string_id_it);
-  return src_mapping;
-}
-
-StackProfileTracker::SourceFrame MakeSourceFrame(
-    const protos::pbzero::Frame::Decoder& entry) {
-  StackProfileTracker::SourceFrame src_frame;
-  src_frame.name_id = entry.function_name_id();
-  src_frame.mapping_id = entry.mapping_id();
-  src_frame.rel_pc = entry.rel_pc();
-  return src_frame;
-}
-
-StackProfileTracker::SourceCallstack MakeSourceCallstack(
-    const protos::pbzero::Callstack::Decoder& entry) {
-  StackProfileTracker::SourceCallstack src_callstack;
-  for (auto frame_it = entry.frame_ids(); frame_it; ++frame_it)
-    src_callstack.emplace_back(*frame_it);
-  return src_callstack;
-}
-
-class ProfilePacketInternLookup : public StackProfileTracker::InternLookup {
- public:
-  ProfilePacketInternLookup(PacketSequenceStateGeneration* seq_state)
-      : seq_state_(seq_state) {}
-
-  base::Optional<base::StringView> GetString(
-      StackProfileTracker::SourceStringId iid,
-      StackProfileTracker::InternedStringType type) const override {
-    protos::pbzero::InternedString::Decoder* decoder = nullptr;
-    switch (type) {
-      case StackProfileTracker::InternedStringType::kBuildId:
-        decoder = seq_state_->LookupInternedMessage<
-            protos::pbzero::InternedData::kBuildIdsFieldNumber,
-            protos::pbzero::InternedString>(iid);
-        break;
-      case StackProfileTracker::InternedStringType::kFunctionName:
-        decoder = seq_state_->LookupInternedMessage<
-            protos::pbzero::InternedData::kFunctionNamesFieldNumber,
-            protos::pbzero::InternedString>(iid);
-        break;
-      case StackProfileTracker::InternedStringType::kMappingPath:
-        decoder = seq_state_->LookupInternedMessage<
-            protos::pbzero::InternedData::kMappingPathsFieldNumber,
-            protos::pbzero::InternedString>(iid);
-        break;
-    }
-    if (!decoder)
-      return base::nullopt;
-    return base::StringView(reinterpret_cast<const char*>(decoder->str().data),
-                            decoder->str().size);
-  }
-
-  base::Optional<StackProfileTracker::SourceMapping> GetMapping(
-      StackProfileTracker::SourceMappingId iid) const override {
-    auto* decoder = seq_state_->LookupInternedMessage<
-        protos::pbzero::InternedData::kMappingsFieldNumber,
-        protos::pbzero::Mapping>(iid);
-    if (!decoder)
-      return base::nullopt;
-    return MakeSourceMapping(*decoder);
-  }
-
-  base::Optional<StackProfileTracker::SourceFrame> GetFrame(
-      StackProfileTracker::SourceFrameId iid) const override {
-    auto* decoder = seq_state_->LookupInternedMessage<
-        protos::pbzero::InternedData::kFramesFieldNumber,
-        protos::pbzero::Frame>(iid);
-    if (!decoder)
-      return base::nullopt;
-    return MakeSourceFrame(*decoder);
-  }
-
-  base::Optional<StackProfileTracker::SourceCallstack> GetCallstack(
-      StackProfileTracker::SourceCallstackId iid) const override {
-    auto* decoder = seq_state_->LookupInternedMessage<
-        protos::pbzero::InternedData::kCallstacksFieldNumber,
-        protos::pbzero::Callstack>(iid);
-    if (!decoder)
-      return base::nullopt;
-    return MakeSourceCallstack(*decoder);
-  }
-
- private:
-  PacketSequenceStateGeneration* seq_state_;
-};
-
-}  // namespace
-
 ProtoTraceParser::ProtoTraceParser(TraceProcessorContext* context)
     : context_(context),
       metatrace_id_(context->storage->InternString("metatrace")),
@@ -225,11 +124,6 @@
                        packet.profile_packet());
   }
 
-  if (packet.has_streaming_profile_packet()) {
-    ParseStreamingProfilePacket(data->sequence_state,
-                                packet.streaming_profile_packet());
-  }
-
   if (packet.has_perf_sample()) {
     ParsePerfSample(ts, data->sequence_state, packet.perf_sample());
   }
@@ -351,14 +245,16 @@
 
   for (auto it = packet.mappings(); it; ++it) {
     protos::pbzero::Mapping::Decoder entry(*it);
-    StackProfileTracker::SourceMapping src_mapping = MakeSourceMapping(entry);
+    StackProfileTracker::SourceMapping src_mapping =
+        ProfilePacketUtils::MakeSourceMapping(entry);
     sequence_state->state()->stack_profile_tracker().AddMapping(entry.iid(),
                                                                 src_mapping);
   }
 
   for (auto it = packet.frames(); it; ++it) {
     protos::pbzero::Frame::Decoder entry(*it);
-    StackProfileTracker::SourceFrame src_frame = MakeSourceFrame(entry);
+    StackProfileTracker::SourceFrame src_frame =
+        ProfilePacketUtils::MakeSourceFrame(entry);
     sequence_state->state()->stack_profile_tracker().AddFrame(entry.iid(),
                                                               src_frame);
   }
@@ -366,7 +262,7 @@
   for (auto it = packet.callstacks(); it; ++it) {
     protos::pbzero::Callstack::Decoder entry(*it);
     StackProfileTracker::SourceCallstack src_callstack =
-        MakeSourceCallstack(entry);
+        ProfilePacketUtils::MakeSourceCallstack(entry);
     sequence_state->state()->stack_profile_tracker().AddCallstack(
         entry.iid(), src_callstack);
   }
@@ -428,50 +324,6 @@
   }
 }
 
-void ProtoTraceParser::ParseStreamingProfilePacket(
-    PacketSequenceStateGeneration* sequence_state,
-    ConstBytes blob) {
-  protos::pbzero::StreamingProfilePacket::Decoder packet(blob.data, blob.size);
-
-  ProcessTracker* procs = context_->process_tracker.get();
-  TraceStorage* storage = context_->storage.get();
-  StackProfileTracker& stack_profile_tracker =
-      sequence_state->state()->stack_profile_tracker();
-  ProfilePacketInternLookup intern_lookup(sequence_state);
-
-  uint32_t pid = static_cast<uint32_t>(sequence_state->state()->pid());
-  uint32_t tid = static_cast<uint32_t>(sequence_state->state()->tid());
-  UniqueTid utid = procs->UpdateThread(tid, pid);
-
-  auto timestamp_it = packet.timestamp_delta_us();
-  for (auto callstack_it = packet.callstack_iid(); callstack_it;
-       ++callstack_it, ++timestamp_it) {
-    if (!timestamp_it) {
-      context_->storage->IncrementStats(stats::stackprofile_parser_error);
-      PERFETTO_ELOG(
-          "StreamingProfilePacket has less callstack IDs than timestamps!");
-      break;
-    }
-
-    auto opt_cs_id = stack_profile_tracker.FindOrInsertCallstack(
-        *callstack_it, &intern_lookup);
-    if (!opt_cs_id) {
-      context_->storage->IncrementStats(stats::stackprofile_parser_error);
-      PERFETTO_ELOG("StreamingProfilePacket referencing invalid callstack!");
-      continue;
-    }
-
-    int64_t ts = sequence_state->state()->IncrementAndGetTrackEventTimeNs(
-        *timestamp_it * 1000);
-    auto trace_ts = context_->clock_tracker->ToTraceTime(
-        protos::pbzero::ClockSnapshot::Clock::MONOTONIC, ts);
-    if (trace_ts)
-      ts = *trace_ts;
-    tables::CpuProfileStackSampleTable::Row sample_row{ts, *opt_cs_id, utid};
-    storage->mutable_cpu_profile_stack_sample_table()->Insert(sample_row);
-  }
-}
-
 void ProtoTraceParser::ParsePerfSample(
     int64_t ts,
     PacketSequenceStateGeneration* sequence_state,
diff --git a/src/trace_processor/importers/proto/proto_trace_parser.h b/src/trace_processor/importers/proto/proto_trace_parser.h
index 3f65fab..ec2cd12 100644
--- a/src/trace_processor/importers/proto/proto_trace_parser.h
+++ b/src/trace_processor/importers/proto/proto_trace_parser.h
@@ -66,7 +66,6 @@
                           PacketSequenceStateGeneration*,
                           uint32_t seq_id,
                           ConstBytes);
-  void ParseStreamingProfilePacket(PacketSequenceStateGeneration*, ConstBytes);
   void ParsePerfSample(int64_t ts, PacketSequenceStateGeneration*, ConstBytes);
   void ParseChromeBenchmarkMetadata(ConstBytes);
   void ParseChromeEvents(int64_t ts, ConstBytes);
diff --git a/src/trace_processor/importers/proto/proto_trace_parser_unittest.cc b/src/trace_processor/importers/proto/proto_trace_parser_unittest.cc
index 5aa0480..00ce4fe 100644
--- a/src/trace_processor/importers/proto/proto_trace_parser_unittest.cc
+++ b/src/trace_processor/importers/proto/proto_trace_parser_unittest.cc
@@ -19,17 +19,15 @@
 #include "perfetto/base/logging.h"
 #include "perfetto/ext/base/string_view.h"
 #include "perfetto/protozero/scattered_heap_buffer.h"
+#include "src/trace_processor/additional_modules.h"
 #include "src/trace_processor/args_tracker.h"
 #include "src/trace_processor/clock_tracker.h"
+#include "src/trace_processor/default_modules.h"
 #include "src/trace_processor/event_tracker.h"
-#include "src/trace_processor/importers/ftrace/ftrace_module.h"
 #include "src/trace_processor/importers/ftrace/sched_event_tracker.h"
-#include "src/trace_processor/importers/proto/proto_importer_module.h"
 #include "src/trace_processor/importers/proto/proto_trace_parser.h"
-#include "src/trace_processor/importers/proto/track_event_module.h"
 #include "src/trace_processor/metadata_tracker.h"
 #include "src/trace_processor/process_tracker.h"
-#include "src/trace_processor/register_additional_modules.h"
 #include "src/trace_processor/slice_tracker.h"
 #include "src/trace_processor/stack_profile_tracker.h"
 #include "src/trace_processor/storage/metadata.h"
@@ -219,8 +217,8 @@
     context_.clock_tracker.reset(clock_);
     context_.sorter.reset(new TraceSorter(&context_, 0 /*window size*/));
     context_.parser.reset(new ProtoTraceParser(&context_));
-    context_.modules.emplace_back(new TrackEventModule(&context_));
 
+    RegisterDefaultModules(&context_);
     RegisterAdditionalModules(&context_);
   }
 
diff --git a/src/trace_processor/trace_processor_impl.cc b/src/trace_processor/trace_processor_impl.cc
index ba45fae..4a43cb9 100644
--- a/src/trace_processor/trace_processor_impl.cc
+++ b/src/trace_processor/trace_processor_impl.cc
@@ -23,9 +23,9 @@
 #include "perfetto/base/time.h"
 #include "perfetto/ext/base/string_splitter.h"
 #include "perfetto/ext/base/string_utils.h"
+#include "src/trace_processor/additional_modules.h"
 #include "src/trace_processor/importers/ftrace/sched_event_tracker.h"
 #include "src/trace_processor/metadata_tracker.h"
-#include "src/trace_processor/register_additional_modules.h"
 #include "src/trace_processor/sql_stats_table.h"
 #include "src/trace_processor/sqlite/span_join_operator_table.h"
 #include "src/trace_processor/sqlite/sqlite3_str_split.h"
diff --git a/src/trace_processor/trace_processor_storage_impl.cc b/src/trace_processor/trace_processor_storage_impl.cc
index c304bec..f462df9 100644
--- a/src/trace_processor/trace_processor_storage_impl.cc
+++ b/src/trace_processor/trace_processor_storage_impl.cc
@@ -19,13 +19,11 @@
 #include "perfetto/base/logging.h"
 #include "src/trace_processor/args_tracker.h"
 #include "src/trace_processor/clock_tracker.h"
+#include "src/trace_processor/default_modules.h"
 #include "src/trace_processor/event_tracker.h"
 #include "src/trace_processor/forwarding_trace_parser.h"
 #include "src/trace_processor/heap_profile_tracker.h"
-#include "src/trace_processor/importers/ftrace/ftrace_module.h"
-#include "src/trace_processor/importers/proto/proto_importer_module.h"
 #include "src/trace_processor/importers/proto/proto_trace_tokenizer.h"
-#include "src/trace_processor/importers/proto/track_event_module.h"
 #include "src/trace_processor/metadata_tracker.h"
 #include "src/trace_processor/perf_sample_tracker.h"
 #include "src/trace_processor/process_tracker.h"
@@ -52,13 +50,7 @@
   context_.global_args_tracker.reset(new GlobalArgsTracker(&context_));
   context_.perf_sample_tracker_.reset(new PerfSampleTracker(&context_));
 
-  context_.modules.emplace_back(new FtraceModule());
-  // Ftrace module is special, because it has one extra method for parsing
-  // ftrace packets. So we need to store a pointer to it separately.
-  context_.ftrace_module =
-      static_cast<FtraceModule*>(context_.modules.back().get());
-
-  context_.modules.emplace_back(new TrackEventModule(&context_));
+  RegisterDefaultModules(&context_);
 }
 
 TraceProcessorStorageImpl::~TraceProcessorStorageImpl() {}