Chromium Code Reviews Chromium Code Reviews
Issue
1629393003:
Reland of [tracing] Dump child processes' memory metrics in browser (Closed)
DescriptionReland 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
Committed: https://crrev.com/e5fb5cd028dfd11dec74ffe183f4b5f360047989
Cr-Commit-Position: refs/heads/master@{#371495}
Patch Set 1 #Messages
Total messages: 18 (6 generated)
|
