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

Issue 827196 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
OOO until Feb 4th
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Supersize: Add source maps for very large .other symbols and res/ dir.

Project Member Reported by wnwen@chromium.org, Mar 29 2018

Issue description

Big one: icudtl.dat, should be in its own place.
Smaller ones: res/*, should be mapped to where it came from.
Others: Optional
 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 11 2018

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

commit 51b9bbb498b269702f769d2326af11bd13f0e219
Author: Peter Wen <wnwen@chromium.org>
Date: Wed Apr 11 23:26:43 2018

Supersize: Add sources for large .other symbols

These large binary files are copied over to the root output directory
and then copied into the apk. There is no good way to propagate their
source paths like pak or dex since they are plainly copied.

These files rarely, if ever, change. Some are even renamed from the
actual source file to this final name. Thus adding them as constants.

Bug:  827196 
Change-Id: I20611da9784fc9b355cc29845a61c3814306e291
Reviewed-on: https://chromium-review.googlesource.com/1007884
Commit-Queue: Peter Wen <wnwen@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549970}
[modify] https://crrev.com/51b9bbb498b269702f769d2326af11bd13f0e219/tools/binary_size/libsupersize/archive.py
[modify] https://crrev.com/51b9bbb498b269702f769d2326af11bd13f0e219/tools/binary_size/libsupersize/integration_test.py
[modify] https://crrev.com/51b9bbb498b269702f769d2326af11bd13f0e219/tools/binary_size/libsupersize/testdata/Archive_Apk.golden

Comment 2 by wnwen@chromium.org, Apr 12 2018

Status: Fixed (was: Assigned)
res/* and the rest of .other symbols are relatively small. Leaving those under *.apk.
Cc: wnwen@chromium.org
Owner: ----
Status: Available (was: Fixed)
Let's re-open for res/. There is potential for much more res/ to be added, and currently it's actually not that small (~1.3mb). Adding source paths should be fairly straight-foward to do in the same way we did pak (each prepare_resources.py invocation can write a .info file).

I agree it's not high priority though.


Comment 4 by wnwen@chromium.org, Apr 16 2018

Summary: Supersize: Add source maps for very large .other symbols and res/ dir. (was: Supersize: Add some source maps for .other symbols)
Sounds good, updated summary to reflect what we want to do.

Comment 5 by wnwen@chromium.org, Apr 16 2018

Cc: -wnwen@chromium.org
Owner: wnwen@chromium.org
Status: Assigned (was: Available)
I would like to actually do this earlier since we want to stable mappings earlier to compare between milestones.
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 19 2018

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

commit da8025abca3ac835c8f59f4622372d4dec02fcae
Author: Peter Wen <wnwen@chromium.org>
Date: Thu Apr 19 17:57:10 2018

Supersize: Properly attribute res/ symbols

Create .info files for resources during jinja processing, aapt2
compressing, and aapt2 linking, using these .info files to create a
per-apk *.apk.res.info file that supersize then uses to properly map a
resource file back to its source file.

There is some complexity with resource overloading and android build
tools changing the resource file or directory as part of packaging an
apk. This is handled by a combination of storing renames and parsing
heuristics.

Bug:  827196 
Change-Id: Ic8243c218791ec048b1563604d93c7b735fdc71c
Reviewed-on: https://chromium-review.googlesource.com/1014255
Commit-Queue: Peter Wen <wnwen@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552081}
[modify] https://crrev.com/da8025abca3ac835c8f59f4622372d4dec02fcae/build/android/gyp/apkbuilder.py
[modify] https://crrev.com/da8025abca3ac835c8f59f4622372d4dec02fcae/build/android/gyp/compile_resources.py
[modify] https://crrev.com/da8025abca3ac835c8f59f4622372d4dec02fcae/build/android/gyp/generate_v14_compatible_resources.py
[modify] https://crrev.com/da8025abca3ac835c8f59f4622372d4dec02fcae/build/android/gyp/jinja_template.py
[modify] https://crrev.com/da8025abca3ac835c8f59f4622372d4dec02fcae/build/android/gyp/prepare_resources.py
[modify] https://crrev.com/da8025abca3ac835c8f59f4622372d4dec02fcae/build/android/gyp/util/resource_utils.py
[modify] https://crrev.com/da8025abca3ac835c8f59f4622372d4dec02fcae/build/config/android/internal_rules.gni
[modify] https://crrev.com/da8025abca3ac835c8f59f4622372d4dec02fcae/tools/binary_size/libsupersize/apkanalyzer.py
[modify] https://crrev.com/da8025abca3ac835c8f59f4622372d4dec02fcae/tools/binary_size/libsupersize/archive.py
[modify] https://crrev.com/da8025abca3ac835c8f59f4622372d4dec02fcae/tools/binary_size/libsupersize/integration_test.py
[modify] https://crrev.com/da8025abca3ac835c8f59f4622372d4dec02fcae/tools/binary_size/libsupersize/models.py
[modify] https://crrev.com/da8025abca3ac835c8f59f4622372d4dec02fcae/tools/binary_size/libsupersize/testdata/Archive_Apk.golden
[add] https://crrev.com/da8025abca3ac835c8f59f4622372d4dec02fcae/tools/binary_size/libsupersize/testdata/mock_output_directory/size-info/test.apk.res.info

Comment 7 by wnwen@chromium.org, Apr 19 2018

Status: Fixed (was: Assigned)

Sign in to add a comment