[tracing] Move track event sessions tracking to trace_startup [chromium/src : main]
0 views
Skip to first unread message
Etienne Pierre-Doray (Gerrit)
Dec 8, 2025, 3:40:42 PM (12 days ago) Dec 8
to Mikhail Khokhlov, Chromium LUCI CQ, chromium...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Attention needed from Mikhail Khokhlov
Etienne Pierre-Doray added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Related details
Attention is currently required from:
- Mikhail Khokhlov
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mikhail Khokhlov (Gerrit)
Dec 9, 2025, 9:23:49 AM (11 days ago) Dec 9
to Etienne Pierre-Doray, Chromium LUCI CQ, chromium...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Attention needed from Etienne Pierre-Doray
Mikhail Khokhlov voted and added 1 comment![]( "Open in Gerrit")
Votes added by Mikhail Khokhlov
| Code-Review | +1 |
1 comment
File base/trace_event/trace_log.h
Line 176, Patchset 5 (Latest):
void OnSetup(const perfetto::DataSourceBase::SetupArgs&) override;Mikhail Khokhlov . unresolved
nit: remove, since no longer needed? (TrackEventSessionObserver has a default implementation that does nothing)
Related details
Attention is currently required from:
- Etienne Pierre-Doray
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Etienne Pierre-Doray (Gerrit)
Dec 9, 2025, 9:52:58 AM (11 days ago) Dec 9
to Mikhail Khokhlov, Chromium LUCI CQ, chromium...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Etienne Pierre-Doray added 1 comment![]( "Open in Gerrit")
File base/trace_event/trace_log.h
Line 176, Patchset 5:
void OnSetup(const perfetto::DataSourceBase::SetupArgs&) override;Mikhail Khokhlov . resolved
nit: remove, since no longer needed? (TrackEventSessionObserver has a default implementation that does nothing)
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Etienne Pierre-Doray (Gerrit)
Dec 9, 2025, 9:53:02 AM (11 days ago) Dec 9
to Mikhail Khokhlov, Chromium LUCI CQ, chromium...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Etienne Pierre-Doray voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Dec 9, 2025, 12:23:51 PM (11 days ago) Dec 9
to Etienne Pierre-Doray, Mikhail Khokhlov, chromium...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
5 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: base/trace_event/trace_log.h
Insertions: 0, Deletions: 1.
@@ -173,7 +173,6 @@
const perfetto::TraceConfig& perfetto_config);
// perfetto::TrackEventSessionObserver implementation.
- void OnSetup(const perfetto::DataSourceBase::SetupArgs&) override;
void OnStart(const perfetto::DataSourceBase::StartArgs&) override;
void OnStop(const perfetto::DataSourceBase::StopArgs&) override;
```
```
The name of the file: base/trace_event/trace_log.cc
Insertions: 0, Deletions: 2.
@@ -700,8 +700,6 @@
return enabled_state_observers_.size();
}
-void TraceLog::OnSetup(const perfetto::DataSourceBase::SetupArgs& args) {}
-
void TraceLog::OnStart(const perfetto::DataSourceBase::StartArgs&) {
{
AutoLock lock(track_event_lock_);
```
Change information
Commit message:
[tracing] Move track event sessions tracking to trace_startup
TraceLog records track event sessions; The only use case for this is
CreateTracingConfigSharedMemory().
Meanwhile, the impl is flawed because it doesn't consider whether
data source is started or not.
This CL moves the tracking to its own class in trace_startup.cc, with
more dedicated tracking, simplifying CreateTracingConfigSharedMemory().
Reason:
- Avoid any new call to GetTrackEventSessions()
- One step towards making TraceLog test only.
This removes dcheck only condition in CreateTracingOutputSharedMemory(),
which isn't guaranteed because session can racily stop.
Change-Id: I871a76e725907cd152a103e1d2dfaff1eed7856a
Reviewed-by: Mikhail Khokhlov <khok...@google.com>
Commit-Queue: Etienne Pierre-Doray <etie...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1556203}
Files:
- M base/trace_event/trace_log.cc
- M base/trace_event/trace_log.h
- M services/tracing/public/cpp/trace_startup.cc
- M services/tracing/public/cpp/trace_startup.h
Change size: M
Delta: 4 files changed, 70 insertions(+), 69 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Mikhail Khokhlov
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages