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

Issue 887357 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 1
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

13.1% regression in loading.mobile at 592220:592304

Project Member Reported by alexclarke@chromium.org, Sep 20

Issue description

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

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


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

android-nexus5x-perf

loading.mobile - Benchmark documentation link:
  https://bit.ly/loading-benchmarks
Cc: wanderview@chromium.org
Owner: wanderview@chromium.org
Status: Assigned (was: Untriaged)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/14bb6f38e40000

Fix blob size usage in ServiceWorkerSubresourceLoader. by wanderview@chromium.org
https://chromium.googlesource.com/chromium/src/+/d31d41fc3662c8ec7d5e334f475222b6e32f678f
1059 → 1224 (+164.4)

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

Benchmark documentation link:
  https://bit.ly/loading-benchmarks
So this bug is saying that removing over-allocation of shared memory when loading with a service worker made flipkart load more slowly.  That is a bit confusing.

I wonder if android's shared memory allocator suffers from fragmentation.  I guess I could try aligning allocations to see if that helps (at the cost of wasted memory).
😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/17581547640000

All of the runs failed. The most common error (20/20 runs) was:
BuildError: Build failed: BUILD_FAILURE
📍 Couldn't reproduce a difference.
https://pinpoint-dot-chromeperf.appspot.com/job/11ec6f97640000
📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/13374c2b640000
😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/17e9b988e40000

All of the runs failed. The most common error (1/20 runs) was:
IOError: [Errno 2] No such file or directory: '/b/swarming/w/itiKJaNP/tmptpoRbNtelemetry/histograms.json'
This pinpoint job suggests aligning pipe sizes to 4kb has some benefits.  Lets see if I can reproduce that and if it has any other effects.
Cc: kmarshall@chromium.org
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/1415a42f640000

[fuchsia] Add support for on-screen keyboard input. by kmarshall@chromium.org
https://chromium.googlesource.com/chromium/src/+/dbfda411da390ce3923a6ea935c3fa056143d3c9
No values → 1061

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

Benchmark documentation link:
  https://bit.ly/loading-benchmarks
Cc: -kmarshall@chromium.org
Sorry kmarshall@, I think that pinpoint job found the wrong commit due to build failures on earlier jobs.
📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/17c5861f640000
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/155fff97640000

Fix blob size usage in ServiceWorkerSubresourceLoader. by wanderview@chromium.org
https://chromium.googlesource.com/chromium/src/+/d31d41fc3662c8ec7d5e334f475222b6e32f678f
1100 → 1245 (+144.9)

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

Benchmark documentation link:
  https://bit.ly/loading-benchmarks
Cc: jordynass@chromium.org
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/1302ee0f640000

[CrOS MultiDevice] Setup Flow UI Styling (cont.) by jordynass@chromium.org
https://chromium.googlesource.com/chromium/src/+/8868e9a206090922044ae8d8a79dc2b8367dc7f4
1040 → No values

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

Benchmark documentation link:
  https://bit.ly/loading-benchmarks
Cc: -jordynass@chromium.org
Sorry.  Another flakey pinpoint result.
Project Member

Comment 24 by bugdroid1@chromium.org, Sep 25

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

commit 5d14fa233a666ecead9d0ef1512a7db36e5244cd
Author: Ben Kelly <wanderview@chromium.org>
Date: Tue Sep 25 18:04:46 2018

Align ashmem_create_region() allocations.

The ashmem_create_region() API documentation indicates the size should
be specified in "page-aligned bytes".

Bug:  887357 
Change-Id: I874914f57408663a380d39d61fddbaf986e6ff24
Reviewed-on: https://chromium-review.googlesource.com/1238673
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Alexandr Ilin <alexilin@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594003}
[modify] https://crrev.com/5d14fa233a666ecead9d0ef1512a7db36e5244cd/base/memory/platform_shared_memory_region_android.cc
[modify] https://crrev.com/5d14fa233a666ecead9d0ef1512a7db36e5244cd/base/memory/shared_memory_android.cc

Status: Fixed (was: Assigned)
I'm going to declare this fixed.  The FlipKart.hot graph still has more noise than previously, but the average has drifted back down to approximately what it was in early September.  Also, some of the other loading.mobile hot graphs show improvement:

https://chromeperf.appspot.com/report?sid=0658ad4cc0587885d2695fa15ba78eb48f30de07868e68207057774156d2dec3

Since we don't have an alert or pinpoint showing this was the improvement we could also mark this WONTFIX.  Since we landed something, though, I'm going to use FIXED.

Sign in to add a comment