New issue
Advanced search Search tips

Issue 818667 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug-Regression



Sign in to add a comment

1%-3.4% regression in system_health.memory_mobile at 539608:539707

Project Member Reported by rmcilroy@chromium.org, Mar 5 2018

Issue description

See the link to graphs below.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=818667

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=c2b0a4b6b1b027bf929a6a0507a93f839cc14d5b020d017463daa31b5b713401


Bot(s) for this bug's original alert(s):

android-webview-nexus5X
android-webview-nexus6
Cc: dtapu...@chromium.org boliu@chromium.org
Owner: dtapu...@chromium.org
Status: Assigned (was: Untriaged)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/1489c2a2440000

Reland "Enable mojo based synchronous compositor" by dtapuska@chromium.org
https://chromium.googlesource.com/chromium/src/+/837639b937d3d1a084c246258ad1f460e4f0f68e

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions

Comment 4 by boliu@chromium.org, Mar 6 2018

hmm, there are more tile memory used after mojo is enabled. that's... unexpected

Comment 5 by boliu@chromium.org, Mar 6 2018

Components: Internals>Mojo Mobile>WebView
Labels: OS-Android
oh, never mind about cc tile memory. I happened to compare two traces that have different tile memory. Checked a few other traces and tile memory is the same.

Regression appear to be under malloc then. That I don't have much expertise in. Is mojo expected to use more memory than ipc? Could try the heap profiler next I guess: https://chromium.googlesource.com/chromium/src/+/lkcr/docs/memory-infra/heap_profiler.md
I think these metrics are noisy.
1) If you look later in the timeline a number of them are recovered and some are better than before.
2) If you look at the average amongst pages there is no trend with these changes
3) If you look at other pages in the test sets you can see quite a same bit of variability

Yes mojo uses a little more memory because of the de/serialization of the structs and this could lead to more allocs (which are freed).

Comment 7 by boliu@chromium.org, Mar 7 2018

Yeah a lot of the graphs are super noisy, but some are not. Eg the load_news_reddit one at the top looks very clean. But noisy doesn't mean there is no regression.

So this went up roughly 0.5MB of malloc memory. That's pretty huge for IPC serialization/deserialization. There aren't any huge messages in sync compositor, and I assume any memory should be freed immediately once the message is handled? So if it's only serialization, then I don't see how it can regress by that much and show up on the graph at all.

Would be interesting to bisect the improvement later that shows up on pretty much every graph, but I assume that's some unrelated improvement.
Cc: roc...@chromium.org
+rockot

Ken do you have any size estimation of how much it would be on total malloc size a new Mojo Channel to the compositor thread would consume. This would be a new endpoint to the compositor thread that didn't exist before.

Not sure which code gets executed in for creating a new end point but was wondering if there are ring buffers there that would account for some of the increased allocations.
Not entirely sure what I know what you mean by a "Mojo Channel". but if you're talking about a bindings interface, i.e. a message pipe, the cost is negligible: probably on the order of a few hundred bytes per endpoint.

There's no persistent ring buffer used for the pipe. Individual messages are their own transient allocations.

If you're talking about a data pipe then that's different, but you have to explicitly choose the size of a data pipe's ring buffer.
Cc: -roc...@chromium.org
Reverse bisect started here:
Cc: nzolghadr@chromium.org
 Issue 818668  has been merged into this issue.
Status: WontFix (was: Assigned)
This appears to be recovered.

Sign in to add a comment