Chrome tracing for system-wide performance stats.
Trace memory, swap, ZRAM, and disk activity and
place the data in JSON to be displayed by trace-viewer.
BUG=236763
TEST=base_unittests TraceSystemStatsMonitorTest.*
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=223806
7 years, 3 months ago
(2013-09-09 16:15:58 UTC)
#8
darin (slow to review)
https://codereview.chromium.org/22836004/diff/31001/base/debug/trace_event_system_stats_monitor.cc File base/debug/trace_event_system_stats_monitor.cc (right): https://codereview.chromium.org/22836004/diff/31001/base/debug/trace_event_system_stats_monitor.cc#newcode46 base/debug/trace_event_system_stats_monitor.cc:46: system_stats_ = SystemMetrics::Sample(); nit: insert new line before close ...
7 years, 3 months ago
(2013-09-12 20:51:15 UTC)
#9
LGTM, but just one suggestion: https://codereview.chromium.org/22836004/diff/37001/base/debug/trace_event_system_stats_monitor.h File base/debug/trace_event_system_stats_monitor.h (right): https://codereview.chromium.org/22836004/diff/37001/base/debug/trace_event_system_stats_monitor.h#newcode25 base/debug/trace_event_system_stats_monitor.h:25: class BASE_EXPORT TraceEventSystemStatsMonitor Had ...
7 years, 3 months ago
(2013-09-17 04:17:58 UTC)
#11
Also, I'm curious about the overall design here. Are we putting too much tracing code ...
7 years, 3 months ago
(2013-09-17 04:18:54 UTC)
#12
Also, I'm curious about the overall design here. Are we putting too much tracing
code into src/base? Have you all considered a design where src/base has only the
bits that are needed by src/base and the rest gets punted to a higher level
library? I could imagine src/debug might be warranted.
jwmak
https://codereview.chromium.org/22836004/diff/37001/base/debug/trace_event_system_stats_monitor.h File base/debug/trace_event_system_stats_monitor.h (right): https://codereview.chromium.org/22836004/diff/37001/base/debug/trace_event_system_stats_monitor.h#newcode25 base/debug/trace_event_system_stats_monitor.h:25: class BASE_EXPORT TraceEventSystemStatsMonitor On 2013/09/17 04:17:59, darin wrote: > ...
7 years, 3 months ago
(2013-09-17 05:39:38 UTC)
#13
https://codereview.chromium.org/22836004/diff/37001/base/debug/trace_event_sy...
File base/debug/trace_event_system_stats_monitor.h (right):
https://codereview.chromium.org/22836004/diff/37001/base/debug/trace_event_sy...
base/debug/trace_event_system_stats_monitor.h:25: class BASE_EXPORT
TraceEventSystemStatsMonitor
On 2013/09/17 04:17:59, darin wrote:
> Had you considered a shorter name for this class? How about just calling it
> TraceEventSystemMonitor? It seems like "Stats" doesn't add much color to the
> name.
The name could be shortened, but there's already a SystemMonitor class in base
that's not related to this one (the other one monitors subsystems such as power
management, network status etc.)
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jwmak@chromium.org/22836004/48001
7 years, 3 months ago
(2013-09-17 19:06:35 UTC)
#14
7 years, 3 months ago
(2013-09-18 07:31:41 UTC)
#19
On 2013/09/17 05:39:38, jwmak wrote:
>
https://codereview.chromium.org/22836004/diff/37001/base/debug/trace_event_sy...
> File base/debug/trace_event_system_stats_monitor.h (right):
>
>
https://codereview.chromium.org/22836004/diff/37001/base/debug/trace_event_sy...
> base/debug/trace_event_system_stats_monitor.h:25: class BASE_EXPORT
> TraceEventSystemStatsMonitor
> On 2013/09/17 04:17:59, darin wrote:
> > Had you considered a shorter name for this class? How about just calling it
> > TraceEventSystemMonitor? It seems like "Stats" doesn't add much color to the
> > name.
>
> The name could be shortened, but there's already a SystemMonitor class in base
> that's not related to this one (the other one monitors subsystems such as
power
> management, network status etc.)
I see. Since this one is prefixed with TraceEvent, I'm not sure that
it would necessarily feel related to the top-level SystemMonitor class.
What about my other question? Why does this code belong in src/base?
commit-bot: I haz the power
Change committed as 223806
7 years, 3 months ago
(2013-09-18 07:33:30 UTC)
#20
Message was sent while issue was closed.
Change committed as 223806
nduca
I suspect we could put it in content/. I think this is something that just ...
7 years, 3 months ago
(2013-09-18 21:15:59 UTC)
#21
Message was sent while issue was closed.
I suspect we could put it in content/. I think this is something that just
slipped through the cracks in the hurry to get it and the tcmalloc sampler
working.
Issue 22836004: Chrome tracing for system-wide performance stats.
(Closed)
Created 7 years, 4 months ago by jwmak
Modified 7 years, 3 months ago
Reviewers: nduca, Sonny, dsinclair, darin (slow to review)
Base URL: https://chromium.googlesource.com/chromium/src.git@asvalue
Comments: 31