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

Issue 895194 link

Starred by 5 users

Issue metadata

Status: Assigned
Owner:
Last visit > 30 days ago
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Use RELR relocations when using Chromium Linker (100kb - non-monochrome)

Project Member Reported by agrieve@chromium.org, Oct 15

Issue description

This supersedes the idea of compressing relocations ( bug 681933 ).

The proposal for RELR relocations is here:
https://groups.google.com/d/msg/generic-abi/bX460iggiKg/Pi9aSwwABgAJ

For Chrome, this would shrink our .rel.dyn 181025 -> ~80kb, and improve start-up time (55ms to apply relocations vs 15ms on my Moto G first gen).

This shrinks the size of arm64 drastically more (by MBs), but not super relevant because Monochrome is where most 64 bit devices are, and it doesn't use the custom linker.

Note that for Monochrome, we *could* use this newer relocation encoding by applying the reloactions via a static initializer, but that wouldn't be viable due to webview's relro-sharing logic.

Thus, this applies only to libchrome.so (not libmonochrome.so).

LLD supports producing RELR via:
https://reviews.llvm.org/D48247  (--pack-dyn-relocs=relr)

Linker will need to be updated with RELR support by copying from bionic linker:
https://android.googlesource.com/platform/bionic/+/master/linker/linker.cpp 
* look for "relocate_relr()"
 
Cc: digit@chromium.org torne@chromium.org agrieve@chromium.org p...@chromium.org
 Issue 681933  has been merged into this issue.
Cc: -digit@google.com
Owner: digit@google.com
Status: Started (was: Available)
Cc: pasko@chromium.org
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 29

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

commit 076573e47b7231213b0f0cf54fe45dc18831b1e6
Author: David 'Digit' Turner <digit@google.com>
Date: Mon Oct 29 16:42:19 2018

android: crazy_linker: Support RELR relocations.

Supporting RELR relocations, as described in [1] has the potential
to drastically reduce the size of native libraries, while speeding
up loading time (see related bug for details).

This CL adds support for RELR relocations to the crazy linker.
Note that this supports both the Android-specific dynamic table
entries, and the default ones supported by lld.

Note that this only modifies the linker. Another CL is required
to enable RELR relocations when building native libraries for
Chrome.

[1] https://reviews.llvm.org/D48247

BUG=895194
R=pasko@chromium.org, agrieve@chromium.org, torne@chromium.org, pcc@chromium.org

Change-Id: Ia074a2228ac7a81418fe706be6e7e57274ae18a0
Reviewed-on: https://chromium-review.googlesource.com/c/1304484
Commit-Queue: David Turner <digit@chromium.org>
Reviewed-by: Egor Pasko <pasko@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603524}
[modify] https://crrev.com/076573e47b7231213b0f0cf54fe45dc18831b1e6/third_party/android_crazy_linker/BUILD.gn
[modify] https://crrev.com/076573e47b7231213b0f0cf54fe45dc18831b1e6/third_party/android_crazy_linker/src/run_tests.sh
[modify] https://crrev.com/076573e47b7231213b0f0cf54fe45dc18831b1e6/third_party/android_crazy_linker/src/src/crazy_linker_elf_relocations.cpp
[modify] https://crrev.com/076573e47b7231213b0f0cf54fe45dc18831b1e6/third_party/android_crazy_linker/src/src/crazy_linker_elf_relocations.h
[add] https://crrev.com/076573e47b7231213b0f0cf54fe45dc18831b1e6/third_party/android_crazy_linker/src/src/crazy_linker_relr_relocations.cpp
[add] https://crrev.com/076573e47b7231213b0f0cf54fe45dc18831b1e6/third_party/android_crazy_linker/src/src/crazy_linker_relr_relocations.h
[add] https://crrev.com/076573e47b7231213b0f0cf54fe45dc18831b1e6/third_party/android_crazy_linker/src/src/crazy_linker_relr_relocations_unittest.cpp
[modify] https://crrev.com/076573e47b7231213b0f0cf54fe45dc18831b1e6/third_party/android_crazy_linker/src/src/elf_traits.h

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 29

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

commit d481ab5408c923a04fd515d555902e84ba41ff77
Author: David 'Digit' Turner <digit@google.com>
Date: Mon Oct 29 17:07:13 2018

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

This saves about 100 kiB on 32-bit ARM libchrome.so.

This optimization cannot be applied to libmonochrome.so, unfortunately,
because only Android P and above support these at the system linker
level.

BUG=895194
R=agrieve@chromium.org,torne@chromium.org,pasko@chromium.org,pcc@chromium.org

Change-Id: I5c554dc3dbc1b3ea27edb60579a5cc0dcdc7d759
Reviewed-on: https://chromium-review.googlesource.com/c/1304487
Commit-Queue: David Turner <digit@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603532}
[modify] https://crrev.com/d481ab5408c923a04fd515d555902e84ba41ff77/build/config/android/BUILD.gn
[modify] https://crrev.com/d481ab5408c923a04fd515d555902e84ba41ff77/chrome/android/BUILD.gn

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 30

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

commit 0a6967d4e6a581c93e5fcecb70ea8ae5d457bf76
Author: Brian Sheedy <bsheedy@chromium.org>
Date: Tue Oct 30 01:20:28 2018

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

This reverts commit d481ab5408c923a04fd515d555902e84ba41ff77.

Reason for revert: Cause of  https://crbug.com/900049 

Original change's description:
> android: build: Enable RELR packed relocations for libchrome.so
> 
> This saves about 100 kiB on 32-bit ARM libchrome.so.
> 
> This optimization cannot be applied to libmonochrome.so, unfortunately,
> because only Android P and above support these at the system linker
> level.
> 
> BUG=895194
> R=​agrieve@chromium.org,torne@chromium.org,pasko@chromium.org,pcc@chromium.org
> 
> Change-Id: I5c554dc3dbc1b3ea27edb60579a5cc0dcdc7d759
> Reviewed-on: https://chromium-review.googlesource.com/c/1304487
> Commit-Queue: David Turner <digit@chromium.org>
> Reviewed-by: agrieve <agrieve@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#603532}

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

Change-Id: Idc7f5a39c0be7a7b7d2221a1635add4d2d5b292a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 895194
Reviewed-on: https://chromium-review.googlesource.com/c/1307077
Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603719}
[modify] https://crrev.com/0a6967d4e6a581c93e5fcecb70ea8ae5d457bf76/build/config/android/BUILD.gn
[modify] https://crrev.com/0a6967d4e6a581c93e5fcecb70ea8ae5d457bf76/chrome/android/BUILD.gn

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 6

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

commit 774b2fbf9b878fb20d557bc820987bcd12382bf0
Author: David 'Digit' Turner <digit@google.com>
Date: Tue Nov 06 15:20:42 2018

android: Do not disable chromium linker on Android N+.

This was previously disabled because the GVR APK was loading
its library with the system dlopen(), then passing the handle
to code in libchrome.so, which happened to call the crazy linker
dlsym() wrapper with it. This resulted in runtime crashes until
a work-around was added in [1].

The Java check was never removed though, so this CL does it.

[1] https://chromium-review.googlesource.com/885763

BUG=895194
R=agrieve@chromium.org, pasko@chromium.org

Change-Id: If742399452336d349eb763a76357a38a72f6707e
Reviewed-on: https://chromium-review.googlesource.com/c/1319589
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@{#605680}
[modify] https://crrev.com/774b2fbf9b878fb20d557bc820987bcd12382bf0/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 18

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

commit cf14fec1c386dc51a6aec3dcc395f1a3527d71b7
Author: David 'Digit' Turner <digit@google.com>
Date: Sun Nov 18 09:52:05 2018

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}
[modify] https://crrev.com/cf14fec1c386dc51a6aec3dcc395f1a3527d71b7/build/config/android/BUILD.gn
[modify] https://crrev.com/cf14fec1c386dc51a6aec3dcc395f1a3527d71b7/chrome/android/BUILD.gn

Status: Fixed (was: Started)
Project Member

Comment 10 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

Status: Assigned (was: Fixed)
Re-opening since this was reverted due to *really* unexpected private RAM increase. Will look into this in the future.
Owner: digit@chromium.org

Comment 13 by agrieve@chromium.org, Yesterday (40 hours ago)

Labels: binary_size_team_q1_2019

Sign in to add a comment