Replace OptGuideLogging with TRACE_EVENTs in SessionImpl. [chromium/src : main]
0 views
Skip to first unread message
Steven Holte (Gerrit)
Oct 14, 2025, 5:31:55 PMOct 14
to Steven Holte, Mike Wittman, AyeAye, spang...@chromium.org, wfh+...@chromium.org, tracing...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com
Attention needed from Mike Wittman
Steven Holte voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Mike Wittman
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mike Wittman (Gerrit)
Oct 14, 2025, 6:16:45 PMOct 14
to Steven Holte, Mike Wittman, Chromium LUCI CQ, AyeAye, spang...@chromium.org, wfh+...@chromium.org, tracing...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com
Attention needed from Steven Holte
Mike Wittman voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Steven Holte
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Steven Holte (Gerrit)
Oct 15, 2025, 5:01:46 PMOct 15
to Steven Holte, Siddhartha S, Mike Wittman, Chromium LUCI CQ, AyeAye, spang...@chromium.org, wfh+...@chromium.org, tracing...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com
Attention needed from Siddhartha S
Steven Holte voted![]( "Open in Gerrit")
Attention is currently required from:
- Siddhartha S
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Steven Holte (Gerrit)
Oct 15, 2025, 5:02:18 PMOct 15
to Steven Holte, Siddhartha S, Mike Wittman, Chromium LUCI CQ, AyeAye, spang...@chromium.org, wfh+...@chromium.org, tracing...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com
Attention needed from Siddhartha S
Steven Holte added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 4 (Latest):
Steven Holte . resolved
+ssid for base/trace_event/builtin_categories.h
Related details
Attention is currently required from:
- Siddhartha S
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Siddhartha S (Gerrit)
Oct 15, 2025, 6:02:44 PMOct 15
to Steven Holte, Siddhartha S, Mike Wittman, Chromium LUCI CQ, AyeAye, spang...@chromium.org, wfh+...@chromium.org, tracing...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com
Attention needed from Steven Holte
Siddhartha S voted and added 1 comment![]( "Open in Gerrit")
Votes added by Siddhartha S
| Code-Review | +1 |
1 comment
File base/trace_event/builtin_categories.h
Line 200, Patchset 4 (Latest):
perfetto::Category("optimization_guide.debug").SetTags("debug"),Siddhartha S . unresolved
consider making this TRACE_DISABLED_BY_DEFAULT if you think this is going to add a lot of events to trace - maybe 10s of events per second or more, no struct guidelines, just for usability
Related details
Attention is currently required from:
- Steven Holte
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. |
Steven Holte (Gerrit)
Oct 15, 2025, 6:56:55 PMOct 15
to Steven Holte, Siddhartha S, Mike Wittman, Chromium LUCI CQ, AyeAye, spang...@chromium.org, wfh+...@chromium.org, tracing...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com
Steven Holte voted and added 1 comment![]( "Open in Gerrit")
Votes added by Steven Holte
| Commit-Queue | +2 |
1 comment
File base/trace_event/builtin_categories.h
Line 200, Patchset 4 (Latest):
perfetto::Category("optimization_guide.debug").SetTags("debug"),Siddhartha S . resolved
consider making this TRACE_DISABLED_BY_DEFAULT if you think this is going to add a lot of events to trace - maybe 10s of events per second or more, no struct guidelines, just for usability
Steven Holte
IIUC "SetTags("debug") already disables it by default, and is preferred now?
// Prefer using ".debug" suffix along with "debug" tag over the legacy
// `DISABLED_BY_DEFAULT()` when creating new debug categories.
// Example: perfetto::Category("cc.debug").SetTags("debug")
// `TRACE_DISABLED_BY_DEFAULT("my_category")` adds
// `disabled-by-default-my_category` prefix and “slow” tag to the category,
// but it doesn’t align with the naming convention, and makes the call sites
// more cluttered compared to the ".debug" suffix.
// Both "slow" and "debug" tags are disabled by default.
I don't think this will cause an issue either way though, these events probably top out at 5/s-ish anyway.
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)
Oct 15, 2025, 8:38:33 PMOct 15
to Steven Holte, Siddhartha S, Mike Wittman, AyeAye, spang...@chromium.org, wfh+...@chromium.org, tracing...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
Replace OptGuideLogging with TRACE_EVENTs in SessionImpl.
Bug: 422805231
Change-Id: I6a6a6964cbb1f413b56d3f95be03acf9765ae82b
Auto-Submit: Steven Holte <ho...@chromium.org>
Reviewed-by: Siddhartha S <ss...@chromium.org>
Commit-Queue: Steven Holte <ho...@chromium.org>
Reviewed-by: Mike Wittman <wit...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1530560}
Files:
- M base/trace_event/builtin_categories.h
- M components/optimization_guide/core/model_execution/model_broker_client.cc
- M components/optimization_guide/core/model_execution/model_execution_manager.cc
- M components/optimization_guide/core/model_execution/on_device_context.cc
- M components/optimization_guide/core/model_execution/on_device_context.h
- M components/optimization_guide/core/model_execution/on_device_execution.cc
- M components/optimization_guide/core/model_execution/on_device_model_service_controller.cc
- M components/optimization_guide/core/model_execution/on_device_model_service_controller.h
- M components/optimization_guide/core/model_execution/on_device_model_service_controller_unittest.cc
- M components/optimization_guide/core/model_execution/session_impl.cc
Change size: M
Delta: 10 files changed, 92 insertions(+), 139 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Mike Wittman, +1 by Siddhartha S
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages