Issue metadata
Sign in to add a comment
|
1.8%-9.8% regression in system_health.memory_mobile at 609142:609150 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Nov 20
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/119b9e3de40000
,
Nov 20
📍 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
,
Nov 21
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
,
Nov 21
Issue 907101 has been merged into this issue.
,
Nov 21
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. :-(
,
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
,
Nov 21
Issue 907096 has been merged into this issue.
,
Nov 22
Issue should be fixed by the revert.
,
Dec 3
Issue 907105 has been merged into this issue. |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Nov 20