New issue
Advanced search Search tips

Issue 907092 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 22
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

1.8%-9.8% regression in system_health.memory_mobile at 609142:609150

Project Member Reported by mustaq@chromium.org, Nov 20

Issue description

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

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


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

Android Nexus5 Perf
android-nexus5x-perf

system_health.memory_mobile - Benchmark documentation link:
  https://bit.ly/system-health-benchmarks

memory.top_10_mobile - Benchmark documentation link:
  None
Cc: digit@google.com
Owner: digit@google.com
Status: Assigned (was: Untriaged)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/119b9e3de40000

android: build: Enable RELR packed relocations for libchrome.so by digit@google.com
https://chromium.googlesource.com/chromium/src/+/cf14fec1c386dc51a6aec3dcc395f1a3527d71b7
memory:chrome:all_processes:reported_by_os:system_memory:private_dirty_size: 3.408e+07 → 3.72e+07 (+3.118e+06)

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

Benchmark documentation link:
  https://bit.ly/system-health-benchmarks
Status: Started (was: Assigned)
Thanks for filing this bug. It doesn't really make sense that enabling RELR relocations would increase private RAM usage by 3.5 MiB, unless the static linker does something really stupid. I'll look into it.

What are the chances that this regression comes from one of the src-internal rolls from the same CL range (see [1]) though?

[1] https://chromium.googlesource.com/chromium/src/+log/cf14fec1c386dc51a6aec3dcc395f1a3527d71b7%5E..0046768337a62af8352abe1196be5c4c9fd23a7c?pretty=fuller&n=1000
 Issue 907101  has been merged into this issue.
After reading the Benchmark documentation link, it is clear that the RELR patch seems to be the culprit. I'm going to revert it for now, and will try to understand this regression later. :-(
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 21

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

commit 448a6987232ea8513b21a0135e25daa8663db9fb
Author: David Turner <digit@chromium.org>
Date: Wed Nov 21 12:09:48 2018

Revert "android: build: Enable RELR packed relocations for libchrome.so"

This reverts commit cf14fec1c386dc51a6aec3dcc395f1a3527d71b7.

Reason for revert: Unexpected 3.5 MiB increase of private RAM usage

BUG= 907092 
TBR=agrieve@chromium.org, pasko@chromium.org

Original change's description:
> android: build: Enable RELR packed relocations for libchrome.so
> 
> NOTE: This is a reland of [1], but this one does not touch
> libchromefortest.so which is used on Android N+ during testing,
> and could not be loaded by the system linker when using RELR
> relocations.
> 
> This optimization cannot be applied to libmonochrome.so, unfortunately,
> because only Android P and above support these at the system linker
> level.
> 
> [1] https://chromium-review.googlesource.com/c/1304487
> 
> BUG=895194
> R=​agrieve@chromium.org,torne@chromium.org,pasko@chromium.org,pcc@chromium.org
> 
> Change-Id: If50f607d2e9abbff5ae0c140da342af7b8c78858
> Reviewed-on: https://chromium-review.googlesource.com/c/1317630
> Reviewed-by: agrieve <agrieve@chromium.org>
> Reviewed-by: Egor Pasko <pasko@chromium.org>
> Commit-Queue: David Turner <digit@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#609142}

TBR=pasko@chromium.org,digit@chromium.org,torne@chromium.org,pcc@chromium.org,agrieve@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 895194
Change-Id: Ib796a90aca376042d8b91f2a388060c3738bb096
Reviewed-on: https://chromium-review.googlesource.com/c/1346090
Reviewed-by: David Turner <digit@chromium.org>
Commit-Queue: David Turner <digit@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610005}
[modify] https://crrev.com/448a6987232ea8513b21a0135e25daa8663db9fb/build/config/android/BUILD.gn
[modify] https://crrev.com/448a6987232ea8513b21a0135e25daa8663db9fb/chrome/android/BUILD.gn

 Issue 907096  has been merged into this issue.
Status: Fixed (was: Started)
Issue should be fixed by the revert.
Issue 907105 has been merged into this issue.

Sign in to add a comment