[tracing] Avoid perfetto track aliasing in chrome/browser/page_load_metrics [chromium/src : main]
0 views
Skip to first unread message
Etienne Pierre-Doray (Gerrit)
May 25, 2026, 10:35:41 AMMay 25
to chromium...@chromium.org, bmcquad...@chromium.org, csharris...@chromium.org, speed-metrics...@chromium.org, loading-rev...@chromium.org, speed-metr...@chromium.org
Etienne Pierre-Doray has uploaded the change for review![]( "Open in Gerrit")
Commit message
[tracing] Avoid perfetto track aliasing in chrome/browser/page_load_metrics
perfetto::Track::FromPointer is problematic because pointer reuse can
cause track aliasing in perfetto UI, leading to confusing traces
(this was true of legacy TRACE_EVENT_ASYNC as well).
To avoid these, we have the following alternatives:
1- Using GetNextGlobalTraceId() instead of a pointer id to guarantee uniqueness.
The track will get its name from the first event emitted.
2- Using NamedTrack to scope the pointer to a specific class
(an aliased track would always correctly have the same name). This is more
appropriate when there are several events emitted to the same track without a
clear name. This has the benefit of giving the track a consistent, explicit name.
3- Replace custom track with "sync" instant events with flows. This reduces visual
clutter and retain information about where events originated from.
This CL applies a migration to of one #1 or #2.
Questions for the reviewer:
- Design Fit: Is the choice of a dedicated async track (#1/#2) appropriate, or
would is be suitable to emit "sync" events with flows (#3). As a rule of thumb,
async tracks are appropriate to display a clear async operation
(that could be recorded as UMA), or high level / user visible state.
- Naming: (when applicable) Is the track name I chose appropriate
This CL was uploaded by git cl split.
Bug: 432427382
Change-Id: Icd683cf364e2f2c8f0b359c1a8d06d95472b1544
Change diff
diff --git a/chrome/browser/page_load_metrics/observers/lcp_critical_path_predictor_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/lcp_critical_path_predictor_page_load_metrics_observer.cc
index fa1b5f63..cb8a1eb 100644
--- a/chrome/browser/page_load_metrics/observers/lcp_critical_path_predictor_page_load_metrics_observer.cc
+++ b/chrome/browser/page_load_metrics/observers/lcp_critical_path_predictor_page_load_metrics_observer.cc
@@ -834,10 +834,11 @@
"Blink.LCPP.NavigationToStartPreload.MainFrame.FirstSubresource.Time",
subresource_load_start);
const base::TimeTicks navigation_start = GetDelegate().GetNavigationStart();
- TRACE_EVENT_BEGIN("loading", "NavigationToStartFirstPreload",
- perfetto::Track::FromPointer(this), navigation_start,
- "url", subresource_url);
- TRACE_EVENT_END("loading", perfetto::Track::FromPointer(this),
+ auto track = perfetto::NamedTrack::FromPointer(
+ "NavigationToStartFirstPreload", this);
+ TRACE_EVENT_BEGIN("loading", "NavigationToStartFirstPreload", track,
+ navigation_start, "url", subresource_url);
+ TRACE_EVENT_END("loading", track,
navigation_start + subresource_load_start);
}
base::UmaHistogramMediumTimes(
Change information
Files:
- M chrome/browser/page_load_metrics/observers/lcp_critical_path_predictor_page_load_metrics_observer.cc
Change size: XS
Delta: 1 file changed, 5 insertions(+), 4 deletions(-)
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. |
Etienne Pierre-Doray (Gerrit)
Jun 10, 2026, 1:19:53 PM (yesterday) Jun 10
to Minoru Chikamune, Chromium LUCI CQ, chromium...@chromium.org, bmcquad...@chromium.org, csharris...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org
Attention needed from Minoru Chikamune
Etienne Pierre-Doray voted and added 1 comment![]( "Open in Gerrit")
Votes added by Etienne Pierre-Doray
| Auto-Submit | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Minoru Chikamune
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Minoru Chikamune (Gerrit)
Jun 10, 2026, 10:29:19 PM (22 hours ago) Jun 10
to Etienne Pierre-Doray, Minoru Chikamune, Chromium LUCI CQ, chromium...@chromium.org, bmcquad...@chromium.org, csharris...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org
Attention needed from Etienne Pierre-Doray
Minoru Chikamune voted and added 1 comment![]( "Open in Gerrit")
Votes added by Minoru Chikamune
| Code-Review | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Etienne Pierre-Doray
Submit Requirements:
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)
6:59 AM (13 hours ago) 6:59 AM
to Minoru Chikamune, Chromium LUCI CQ, chromium...@chromium.org, bmcquad...@chromium.org, csharris...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org
Etienne Pierre-Doray voted Commit-Queue+2![]( "Open in Gerrit")
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)
7:30 AM (13 hours ago) 7:30 AM
to Etienne Pierre-Doray, Minoru Chikamune, chromium...@chromium.org, bmcquad...@chromium.org, csharris...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[tracing] Avoid perfetto track aliasing in chrome/browser/page_load_metrics
perfetto::Track::FromPointer is problematic because pointer reuse can
cause track aliasing in perfetto UI, leading to confusing traces (this
was true of legacy TRACE_EVENT_ASYNC as well).
In this case, I removed the events, since NavigationToStartFirstPreload
is deprecated.
Bug: 432427382
Change-Id: Icd683cf364e2f2c8f0b359c1a8d06d95472b1544
Auto-Submit: Etienne Pierre-Doray <etie...@chromium.org>
Reviewed-by: Minoru Chikamune <chik...@chromium.org>
Commit-Queue: Etienne Pierre-Doray <etie...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1645264}
Files:
- M chrome/browser/page_load_metrics/observers/lcp_critical_path_predictor_page_load_metrics_observer.cc
Change size: S
Delta: 1 file changed, 0 insertions(+), 12 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Minoru Chikamune
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages