• ssid's avatar
    Reland of [tracing] Dump child processes' memory metrics in browser (patchset... · e5fb5cd0
    ssid authored
    Reland of [tracing] Dump child processes' memory metrics in browser (patchset #1 id:1 of https://codereview.chromium.org/1617263002/ )
    
    Reason for revert:
    The crash was fixed in crrev.com/1620783002
    
    Original issue's description:
    > Revert of [tracing] Dump child processes' memory metrics in browser (patchset #1 id:1 of https://codereview.chromium.org/1591553002/ )
    >
    > Reason for revert:
    > TracingBrowserTest.TestMemoryInfra is failing on DrMemory bot with the following
    > error:
    >
    > UNADDRESSABLE ACCESS: reading 0x00000048-0x0000004c 4 byte(s)
    > # 0 base.dll!base::Thread::task_runner
    > [base\threading\thread.h:168]
    > # 1 base.dll!base::trace_event::MemoryDumpManager::CreateProcessDump
    > [base\trace_event\memory_dump_manager.cc:313]
    > # 2 content.dll!content::TracingControllerImpl::RequestGlobalMemoryDump
    > [content\browser\tracing\tracing_controller_impl.cc:1039]
    > # 3 content.dll!base::internal::Invoker<>::Run
    > [base\bind_internal.h:350]
    >
    > bug: crbug.com/580295
    >
    > Original issue's description:
    > > Reland of [tracing] Dump child processes' memory metrics in browser (patchset #1 id:1 of https://codereview.chromium.org/1586893003/ )
    > >
    > > The failure was due to flakiness fixed by crrev.com/1606983002.
    > > See crbug.com/578128.
    > >
    > > Original issue's description:
    > > > Revert of [tracing] Dump child processes' memory metrics in browser (patchset #15 id:400001 of https://codereview.chromium.org/1417003003/ )
    > > >
    > > > Reason for revert:
    > > > This is suspected by Findit to cause failure in TracingBrowserTest.TestMemoryInfra under Mac ASan 64 Tests (1).
    > > >
    > > > Original issue's description:
    > > > > [tracing] Dump child processes' memory metrics in browser
    > > > >
    > > > > The sandbox in linux prevents the process from reading the process
    > > > > metrics file. To work around this issue, the browser process will now
    > > > > dump statistics of the child processes too. This requires the
    > > > > composable dumps support in trace viewer and telemetry.
    > > > >
    > > > > This CL makes following changes.
    > > > > 1. Move the process totals and memory maps dump provider into a single
    > > > > header as process_metrics_dump_provider in components/tracing. This is
    > > > > because it is not necessary to have this in base and now the provider
    > > > > knows/manages for different processes. Also the dump method are made to
    > > > > handle error better, since process can vanish while dumping.
    > > > >
    > > > > 2. Make the dump provider non-singleton and have a register / unregister
    > > > > that manages the lifetime of the dump providers.
    > > > >
    > > > > 3. The dump providers unregister using the new
    > > > > UnregisterAndDeleteDumpProviderAsync api added by crrev.com/1430073002.
    > > > >
    > > > > 4. On linux the browser process dumps metrics for all processes and on
    > > > > android the child processes can dump since seccomp sandbox is not
    > > > > enabled in android yet.
    > > > >
    > > > > 5. The proc/status file is human readable stats and is not guaranteed to
    > > > > have a field that is asked for. So, the NOTREACHED is removed in
    > > > > ReadProcStatusAndGetFieldAsSizeT.
    > > > >
    > > > > 6. Since we introduce other process dumps from browser process there
    > > > > could be races while unregistering and dumping. To test this, the
    > > > > browser test is updated.
    > > > >
    > > > > BUG=461788
    > > > >
    > > > > Committed: https://crrev.com/4d77d76a42425282b1a3c5b7309db9b98e777f60
    > > > > Cr-Commit-Position: refs/heads/master@{#369482}
    > > >
    > > > TBR=primiano@chromium.org,thakis@chromium.org,simonhatch@chromium.org,sievers@chromium.org,ssid@chromium.org
    > > > # Skipping CQ checks because original CL landed less than 1 days ago.
    > > > NOPRESUBMIT=true
    > > > NOTREECHECKS=true
    > > > NOTRY=true
    > > > BUG=461788
    > > >
    > > > Committed: https://crrev.com/93aa967cfcb3e933000f169b9a4f7ac84dbd96da
    > > > Cr-Commit-Position: refs/heads/master@{#369535}
    > >
    > > TBR=primiano@chromium.org,thakis@chromium.org,simonhatch@chromium.org,sievers@chromium.org,huangs@chromium.org
    > > BUG=461788
    > > CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
    > >
    > > Committed: https://crrev.com/df4156349708e21844209290a2d3638b550e18b2
    > > Cr-Commit-Position: refs/heads/master@{#370195}
    >
    > TBR=primiano@chromium.org,thakis@chromium.org,simonhatch@chromium.org,sievers@chromium.org,huangs@chromium.org,ssid@chromium.org
    > # Not skipping CQ checks because original CL landed more than 1 days ago.
    > BUG=461788
    >
    > Committed: https://crrev.com/94bef8d8bf199e23102783a55a15c9f10bc88631
    > Cr-Commit-Position: refs/heads/master@{#370836}
    
    TBR=primiano@chromium.org,thakis@chromium.org,simonhatch@chromium.org,sievers@chromium.org,huangs@chromium.org,oshima@chromium.org
    # Not skipping CQ checks because original CL landed more than 1 days ago.
    BUG=461788
    
    Review URL: https://codereview.chromium.org/1629393003
    
    Cr-Commit-Position: refs/heads/master@{#371495}
    e5fb5cd0
process_metrics_linux.cc 30.7 KB