New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 769583 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

OOP HP: Performance issues.

Project Member Reported by erikc...@chromium.org, Sep 28 2017

Issue description

I observe this both with --memlog=minimal and --memlog=all, but more frequently with the latter. Chrome itself seems to have no performance issues, but the profiling process gets "backed up" with allocations.

Example 1: Open Chrome with --memlog=minimal. Hold ctr+tab to quickly move between tabs. Observe that the profiling process is pegged at 100% cpu usage, and its memory grows over time.

The browser process is able to quickly grab stack-traces and ship them over the OS pipe to the profiling process. The profiling process reads the results, and creates async tasks to process the data. These async tasks are queued faster than they are dequeued, resulting in a huge backlog for the profiling process. I've observed the profiling process hit 4GB+ memory usage, before falling down over ~60s to ~1GB. 

I took a sample of a backed-up profiling process. 

Part of the problem is that we have a global lock for backtrace storage, and there is lock contention. Given that we already have a fingerprint for backtraces, let's shard the storage across N unordered_sets? That should have comparable memory usage but solve the lock issue.

The other problem is that a lot of time is spent in tree operations for AllocationEventSet. It's not clear to me why we use a set rather than an unordered_set. The latter will probably have a lot better performance.
 
oop_hp_profiling_process_trace.txt
357 KB View Download
Owner: erikc...@chromium.org
Status: Assigned (was: Untriaged)
I implementing sharding for BAcktraceStorage and can no longer discern any performance issues. Skipping the second proposal.
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3b0a23ceca141150938f663d4ba889ce4e420207

commit 3b0a23ceca141150938f663d4ba889ce4e420207
Author: erikchen <erikchen@chromium.org>
Date: Mon Oct 09 20:57:15 2017

Improve performance of profiling process.

The bulk of the performance overhead comes from lock contention for
BacktraceStorage. This CL shards BacktraceStorage with 64 shards, using the
Backtrace fingerprint to determine which shard to store each Backtrace in.

Bug:  769583 
Change-Id: Id46a36113dd0973d26917fd7a4230b4a9e7272e1
Reviewed-on: https://chromium-review.googlesource.com/688723
Reviewed-by: Brett Wilson <brettw@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507478}
[modify] https://crrev.com/3b0a23ceca141150938f663d4ba889ce4e420207/chrome/profiling/backtrace_storage.cc
[modify] https://crrev.com/3b0a23ceca141150938f663d4ba889ce4e420207/chrome/profiling/backtrace_storage.h

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3823737f4c244e08f72bedf2ca53529ec605cc70

commit 3823737f4c244e08f72bedf2ca53529ec605cc70
Author: erikchen <erikchen@chromium.org>
Date: Fri Oct 20 00:28:51 2017

Disable tracing for profiling service.

Chrome suffers a significant performance degradation when both tracing and
out-of-process heap-profiling are enabled. This is because the profiling service
emits multiple trace events each time it reads from a profiling pipe, which
happens around every 30 allocations or so. For more details, see the thread
https://groups.google.com/a/google.com/forum/#!topic/chrome-benchmarking/rjNyxf_53lU.

Bug:  769583 
Change-Id: Iba006679154037268bb91b5baf09857a73aae8d2
Reviewed-on: https://chromium-review.googlesource.com/723747
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510265}
[modify] https://crrev.com/3823737f4c244e08f72bedf2ca53529ec605cc70/content/child/child_thread_impl.cc
[modify] https://crrev.com/3823737f4c244e08f72bedf2ca53529ec605cc70/content/child/child_thread_impl.h

Status: Fixed (was: Assigned)

Sign in to add a comment