Merge branch 'main' into dev/annabauza/android_users_table_processor

Author: annathedroid

docs/AGENTS.md

@@ -1,23 +1,48 @@
-# Perfetto Project Development Guidelines
+# Perfetto Project Development Guidelines for AI Agents
 
-This document provides essential instructions and best practices for developing
-in the Perfetto codebase. Adhere to these guidelines to ensure consistency and
-quality.
+This document provides essential instructions and best practices for developing in the Perfetto codebase. Adhere to these guidelines to ensure consistency and quality.
 
-## 1. Building the Project
+## Overview
+
+The perfetto repo contains several projects. These are the major ones
+
+1. Tracing services: a set of C++ projects that ship on the target device being traced. They live in src/{traced, traced_probes, tracing}. Target names: perfetto, traced, traced_probes, traced_perf, heapprofd.
+
+2. Tracing SDK: this lives in src/tracing. They are used by apps that want to use Perfetto tro emit trace events. There are two flavours of this SDK.
+   - The (older) C++ SDK: reachable via include/perfetto/tracing/.
+   - The (newer) C SDK: reachable via include/perfetto/public.
+
+3. TraceProcessor: a C++ project that lives in src/trace_processor/. This code is typically NOT shipped on device and is used by offline tools. It is internally based on sqlite and it extends its query engine via the vtable API. The UI uses this building in Wasm (Web Assembly).
+
+4. Perfetto UI: This is a Single Page Web Application, client-only (no server component) written in TypeScript that powers ui.perfetto.dev. It lives in ui/. It embeds TraceProcessor via Wasm.
+
+5. A bunch of other tools and utilities used rarely, in tools/ and src/tools.
+
+## Core Software Engineering Principles
+
+Follow these principles when writing and modifying code.
+
+- Avoid code duplication: Before writing a new function, search the codebase for existing functions that provide similar functionality.
+- Reuse and refactor: If a suitable function exists, reuse it. If it's close but not an exact match, consider refactoring the existing function to accommodate the new use case instead of creating a copy.
+- Consult if unsure: If you are considering duplicating a function or a significant block of code, consult with the user first.
+
+## C++ projects overview
+
+GN supports different configurations, one per out/* folder. You can inspect them by looking at out/xxx/args.gn. Typically when building/running tests while developing, we target our local machine (linux or mac) and we do NOT use android targets.
 
 Use the following commands to build the project for different configurations.
 All commands should be run from the root of the repository.
 
-The output folder where code is built lives in out/xxx. Different people use
-different output folders. Pick the output folder as follows
+The output folder where code is built lives in out/xxx. Different people use different output folders.
+Pick the output folder as follows
+
 - The $OUT env var should contain the path of the output folder. If it exists respect that.
-- If $OUT is empty/unset, set it by looking at the most recent out/ subdir, i.e.
-  `OUT=out/$(ls -t1 out | head -n1)`
+- If $OUT is empty/unset, set it by looking at the most recent out/ subdir, i.e. `export OUT=out/$(ls -t1 out | head -n1)`
 
 ### Building C++ code
 
-Our C++ sources use tools/gn and tools/ninja to build.
+Our primary build system is "gn" (GenerateNinja) + "ninja".
+These tools are checked in and accessible via wrapper scripts in our repo, tools/gn and tools/ninja.
 
 - If you touch a .gn or .gni file, rerun `tools/gn gen --check $OUT`
 - Afterwards, you can build code running `tools/ninja -C $OUT TARGET_NAME"
@@ -31,16 +56,72 @@ When adding/removing source files, keep BUILD.gn files updated.
 Usually there is a BUILD.gn file in each directory. If not, look at closer parent dirs for precedent.
 
 Never bother manually updating Android.bp files or bazel BUILD files.
-Those are autogenerated later when uploading the pull request via
-`tools/gen_all $OUT`, but humans will take care of that.
+Those are autogenerated later when uploading the pull request via `tools/gen_all $OUT`, but humans will take care of that.
 
 To build one or more targets:
 
 ```sh
 tools/ninja -C $OUT -k 10000 trace_processor_shell perfetto_unittests
 ```
 
-### 2. Running C++ Tests
+All the C++ projects share the same "base" target (include/perfetto/base, include/ext/perfetto/base) and can share some other targets (See GN).
+
+### C++ Code style
+
+We mainly adhere to the Google C++ style guide, which you can consult here
+https://google.github.io/styleguide/cppguide.html
+
+Highlights:
+
+- Use C++17.
+- Don't use exceptions, don't bother with try/catch.
+- Try to keep template usage to a minimum.
+- Prefer forward declarations in header files, and #include the needed deps in the .cc files.
+- We fail fast. If something shouldn't happen add a PERFETTO_DCHECK() (debug only) or a PERFETTO_CHECK() (production).
+- Remember to never put code with side effects inside a PERFETTO_DCHECK, as those become no-ops in release builds.
+- If a function argument is out or inout, pass it by pointer, not by reference.
+- Delete copy and move constructors unless they are really needed.
+- If you need a copy/move constructor use the same pattern seen in include/perfetto/ext/base/circular_queue.h
+- Use PERFETTO_DLOG (debug only), PERFETTO_LOG/ILOG/ELOG() for logging.
+- Variable names should be proportional to the scope and distance. Variables used in a small loop can be called i,j; Variables scoped to a function should be shorter; Variables/function used across different files should be longer (within reasonable limits) and less prone to collisions when code-searching.
+- Avoid libraries other than STL, posix/Unix common headers, and other libraries that we already pull in buildtools/. If you think you need a new library, ask the user.
+- Generally, be consistent with the style of the file.
+
+When possible try to use data structures and constructs available in include/perfetto/base/ and include/perfetto/ext/base/. Generally look for precedent in the codebase. If in doubt, ask the .
+
+Frequently used includes you should look into are, under include/:
+
+- perfetto/base/task_runner.h"
+- perfetto/base/compiler.h"
+- perfetto/base/time.h"
+- perfetto/ext/base/status_or.h"
+- perfetto/ext/base/scoped_file.h"
+- perfetto/ext/base/file_utils.h"
+- perfetto/ext/base/flat_hash_map.h"
+- perfetto/ext/base/utils.h"
+- perfetto/ext/base/string_view.h"
+- perfetto/ext/base/string_utils.h"
+- perfetto/base/status.h"
+- perfetto/base/logging.h"
+
+When creating new files, this is where you put headers:
+
+- .cc files always go under src/, with the exception of some test/ code.
+- If possible, keep .h headers private and keep them alongside the .cc file.
+- If needed you can put headers in include/perfetto/ext, as that is a non-public API surface.
+- In rare cases, if the user says so, you can put new headers in include/perfetto/public, but that's only for C-SDK cases.
+- Note that "include/" is in the include path, so you never need to type #include "include/perfetto/foo" but only #include "perfetto/foo".
+
+### Supporting different OSes in C++
+
+We generally support all the major platforms (Linux, Android, MacOS/iOS,  Windows) in our codebase, with the exception of src/traced which is supported only on Linux/Android (and few parts on MacOS).
+
+If you need to split code for different platforms, you must use  perfetto/base/build_config.h, and specifically the macros therein defined as `#if PERFETTO_BUILDFLAG(PERFETTO_OS_ANDROID)` and so on.
+
+Note that every PERFETTO_BUILDFLAG_DEFINE_XXX that you see there must be used via the PERFETTO_BUILDFLAG(XXX) wrapper.
+For example PERFETTO_BUILDFLAG(PERFETTO_OS_QNX) when you see  PERFETTO_BUILDFLAG_DEFINE_PERFETTO_OS_QNX.
+
+### Running C++ Tests
 
 Perfetto uses the Google Test framework. You will see c++ sources like
 ```cpp
@@ -69,55 +150,33 @@ For perfetto_benchmarks you need instead to run
 $OUT/perfetto_benchmarks --benchmark_filter='.*BM_RtMutex_NoContention.*'
 ```
 
-Note that unlike Google Test, where the filter is a glob, in Google Benchmarks the filter is a regex.
+Note that unlike Google Test, where the filter is a glob, in Google Benchmarks
+the filter is a regex.
 
 ### Trace Processor Diff Tests
 
-Trace Processor Diff Tests (or diff tests for short) are executed by running the
-following command:
+Trace Processor Diff Tests (or diff tests for short) are executed by running the following command:
 
 ```sh
 tools/diff_test_trace_processor.py $OUT$/trace_processor_shell --keep-input --quiet --name-filter="<regex of test names>"
 ```
 
-**Note:** These tests can also be run with ASan or MSan builds by changing the
-path from `out/linux_clang_release/` to `out/linux_asan/` or `out/linux_msan/`
-respectively. **Note:** The `--name-filter` argument is optional. **Note:** When
-using the `--name-filter` flag, do not include `test_` in the filter. The test
-runner automatically drops this prefix. For example, to run `test_my_cool_test`,
-use the filter `MyTestSuite.my_cool_test`.
-
+**Note:** These tests can also be run with ASan or MSan builds by changing the path from `out/linux_clang_release/` to `out/linux_asan/` or `out/linux_msan/` respectively.
 
-### Test Guidelines
-
-- **Prefer test suites over individual tests.** When using the `--gtest_filter`
-  flag, specify a whole test suite (e.g., `"MyTestSuite.*"`) instead of a single
-  test case (e.g., `"MyTestSuite.MySpecificTest"`). This ensures broader test
-  coverage.
-- **Do not test unstable IDs.** When writing diff tests, do not include columns
-  that contain unstable IDs (e.g. `upid`, `utid`, `id`, etc) in the output. These
-  IDs can change between different runs of the same test, which will cause the
-  test to fail.
-- **Remove `test_` prefix for diff tests.** When using the `--name-filter` flag
-  for diff tests, do not include `test_` in the filter. The test
-  runner automatically drops this prefix. For example, to run `test_my_cool_test`,
-  use the filter `MyTestSuite.my_cool_test`.
-
-## 3. Core Software Engineering Principles
+**Note:** The `--name-filter` argument is optional.
 
-Follow these principles when writing and modifying code.
+**Note:** When using the `--name-filter` flag, do not include `test_` in the filter. The test
+runner automatically drops this prefix.
+For example, to run `test_my_cool_test`, use the filter `MyTestSuite.my_cool_test`.
 
-### Principle 1: Don't Repeat Yourself (DRY)
+### Test Guidelines
 
-- **Avoid code duplication.** Before writing a new function, search the codebase
-  for existing functions that provide similar functionality.
-- **Reuse and refactor.** If a suitable function exists, reuse it. If it's close
-  but not an exact match, consider refactoring the existing function to
-  accommodate the new use case instead of creating a copy.
-- **Consult if unsure.** If you are considering duplicating a function or a
-  significant block of code, consult with the user first.
+- **Prefer test suites over individual tests.** When using the `--gtest_filter` flag, specify a whole test suite (e.g., `"MyTestSuite.*"`) instead of a single test case (e.g., `"MyTestSuite.MySpecificTest"`). This ensures broader test coverage.
+- **Do not test unstable IDs.** When writing diff tests, do not include columns that contain unstable IDs (e.g. `upid`, `utid`, `id`, etc) in the output. These IDs can change between different runs of the same test, which will cause the test to fail.
+  
+- **Remove `test_` prefix for diff tests.** When using the `--name-filter` flag for diff tests, do not include `test_` in the filter. The test runner automatically drops this prefix. For example, to run `test_my_cool_test`, use the filter `MyTestSuite.my_cool_test`.
 
-## 4. Getting Diffs
+## Getting Diffs
 
 When asked to "get a diff" or "read the current diff", run the following
 command:
@@ -126,7 +185,7 @@ command:
 git diff $(git config branch.$(git rev-parse --abbrev-ref HEAD).parent)
 ```
 
-## 5. Fixing GN Dependencies
+## Fixing GN Dependencies
 
 When asked to fix GN dependencies, run the following command and fix any errors
 that are reported:
@@ -139,7 +198,7 @@ tools/gn check $OUT
 unless explicitly instructed to by the user. Instead, add a direct dependency to
 the target that requires it.
 
-## 6. Other Configurations
+## Other Configurations
 
 ### ASan (AddressSanitizer) Build
 
@@ -174,25 +233,21 @@ out/linux_asan/perfetto_unittests --gtest_brief=1 --gtest_filter="<TestSuiteName
 MSAN_SYMBOLIZER_PATH="$(pwd)/buildtools/linux64/clang/bin/llvm-symbolizer" \
 out/linux_msan/perfetto_unittests --gtest_brief=1 --gtest_filter="<TestSuiteName.*>"
 
-## 7. Creating Pull Requests
+## Creating Pull Requests
 
 When creating a pull request, follow these steps:
 
 1.  **Create a new branch:**
-    Use the command `git new-branch dev/lalitm/<name-of-branch>` to create a new branch for your pull request.
+    Use the command `git new-branch dev/$USER$/<name-of-branch>` to create a new branch for your pull request.
 
 2.  **Create a stacked/dependent pull request:**
     To create a pull request that depends on another, use the command `git new-branch --parent <name-of-parent-branch> dev/lalitm/<name-of-branch>`.
 
-**Note:** The `git new-branch` command only creates and switches to a new
-branch. The normal `git add` and `git commit` workflow should be used to add
-changes to the branch.
+**Note:** The `git new-branch` command only creates and switches to a new branch. The normal `git add` and `git commit` workflow should be used to add changes to the branch.
 
-## 8. Commit Messages
+## Commit Messages
 
 When writing commit messages, follow these guidelines:
 
-- **Prefix your commits.** Prefix changes to Trace Processor code with `tp:`,
-  UI code with `ui:`, and general Perfetto changes with `perfetto:`.
-- **Keep it concise.** A short one-line summary followed by a paragraph
-  describing the change is the best commit message.
+- Prefix your commits: Prefix changes to Trace Processor code with `tp:`, UI code with `ui:`, and general Perfetto changes with `perfetto:`.
+- Keep it concise: A short one-line summary followed by a \n then a paragraph describing the change is the best commit message. Wrap the commit message at 72 cols.

include/perfetto/trace_processor/trace_processor.h

@@ -46,6 +46,33 @@ class PERFETTO_EXPORT_COMPONENT TraceProcessor : public TraceProcessorStorage {
 
   ~TraceProcessor() override;
 
+  // =================================================================
+  // |        Trace loading related functionality starts here         |
+  // =================================================================
+
+  // The entry point to push trace data into the processor. The trace format
+  // will be automatically discovered on the first push call. It is possible
+  // to make queries between two pushes.
+  // Returns the Ok status if parsing has been succeeding so far, and Error
+  // status if some unrecoverable error happened. If this happens, the
+  // TraceProcessor will ignore the following Parse() requests, drop data on the
+  // floor and return errors forever.
+  base::Status Parse(TraceBlobView) override = 0;
+
+  // Shorthand for Parse(TraceBlobView(TraceBlob(TakeOwnership(buf, size))).
+  // For compatibility with older API clients.
+  base::Status Parse(std::unique_ptr<uint8_t[]> buf, size_t size);
+
+  // Forces all data in the trace to be pushed to tables without buffering data
+  // in sorting queues. This is useful if queries need to be performed to
+  // compute post-processing data (e.g. deobfuscation, symbolization etc) which
+  // will be appended to the trace in a future call to Parse.
+  void Flush() override = 0;
+
+  // Calls Flush and finishes all of the actions required for parsing the trace.
+  // Calling this function multiple times is undefined behaviour.
+  base::Status NotifyEndOfFile() override = 0;
+
   // =================================================================
   // |        PerfettoSQL related functionality starts here          |
   // =================================================================

include/perfetto/trace_processor/trace_processor_storage.h

@@ -17,14 +17,13 @@
 #ifndef INCLUDE_PERFETTO_TRACE_PROCESSOR_TRACE_PROCESSOR_STORAGE_H_
 #define INCLUDE_PERFETTO_TRACE_PROCESSOR_TRACE_PROCESSOR_STORAGE_H_
 
+#include <cstddef>
 #include <cstdint>
-
 #include <memory>
 
 #include "perfetto/base/export.h"
 #include "perfetto/base/status.h"
 #include "perfetto/trace_processor/basic_types.h"
-#include "perfetto/trace_processor/status.h"
 #include "perfetto/trace_processor/trace_blob_view.h"
 
 namespace perfetto::trace_processor {
@@ -37,27 +36,16 @@ class PERFETTO_EXPORT_COMPONENT TraceProcessorStorage {
 
   virtual ~TraceProcessorStorage();
 
-  // The entry point to push trace data into the processor. The trace format
-  // will be automatically discovered on the first push call. It is possible
-  // to make queries between two pushes.
-  // Returns the Ok status if parsing has been succeeding so far, and Error
-  // status if some unrecoverable error happened. If this happens, the
-  // TraceProcessor will ignore the following Parse() requests, drop data on the
-  // floor and return errors forever.
+  // See comment on TraceProcessor::Parse.
   virtual base::Status Parse(TraceBlobView) = 0;
 
-  // Shorthand for Parse(TraceBlobView(TraceBlob(TakeOwnership(buf, size))).
-  // For compatibility with older API clients.
+  // See comment on TraceProcessor::Parse.
   base::Status Parse(std::unique_ptr<uint8_t[]> buf, size_t size);
 
-  // Forces all data in the trace to be pushed to tables without buffering data
-  // in sorting queues. This is useful if queries need to be performed to
-  // compute post-processing data (e.g. deobfuscation, symbolization etc) which
-  // will be appended to the trace in a future call to Parse.
+  // See comment on TraceProcessor::Flush.
   virtual void Flush() = 0;
 
-  // Calls Flush and finishes all of the actions required for parsing the trace.
-  // Calling this function multiple times is undefined behaviour.
+  // See comment on TraceProcessor::NotifyEndOfFile.
   virtual base::Status NotifyEndOfFile() = 0;
 };
 

src/trace_processor/read_trace_internal.cc

@@ -16,26 +16,27 @@
 
 #include "src/trace_processor/read_trace_internal.h"
 
+#include <fcntl.h>
+#include <algorithm>
+#include <cerrno>
+#include <cstddef>
+#include <cstdint>
+#include <cstdlib>
+#include <cstring>
+#include <functional>
+#include <utility>
+
 #include "perfetto/base/logging.h"
+#include "perfetto/base/status.h"
 #include "perfetto/ext/base/file_utils.h"
 #include "perfetto/ext/base/scoped_file.h"
 #include "perfetto/ext/base/scoped_mmap.h"
-#include "perfetto/ext/base/utils.h"
-#include "perfetto/protozero/proto_utils.h"
-#include "perfetto/trace_processor/trace_processor.h"
-
 #include "perfetto/ext/base/status_macros.h"
 #include "perfetto/trace_processor/trace_blob.h"
 #include "perfetto/trace_processor/trace_blob_view.h"
-#include "src/trace_processor/forwarding_trace_parser.h"
-#include "src/trace_processor/importers/proto/proto_trace_tokenizer.h"
-#include "src/trace_processor/util/gzip_utils.h"
-
-#include "protos/perfetto/trace/trace.pbzero.h"
-#include "protos/perfetto/trace/trace_packet.pbzero.h"
+#include "perfetto/trace_processor/trace_processor.h"
 
-namespace perfetto {
-namespace trace_processor {
+namespace perfetto::trace_processor {
 namespace {
 
 // 1MB chunk size seems the best tradeoff on a MacBook Pro 2013 - i7 2.8 GHz.
@@ -112,5 +113,4 @@ base::Status ReadTraceUnfinalized(
     progress_callback(bytes_read);
   return base::OkStatus();
 }
-}  // namespace trace_processor
-}  // namespace perfetto
+}  // namespace perfetto::trace_processor