Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(316)

Side by Side Diff: tools/perf/docs/apk_size_regressions.md

Issue 2675973002: Tweak apk_size_regressions.md to be more explicit about a couple things (Closed)
Patch Set: add bug link for template Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 # How to Deal with Android Size Alerts 1 # How to Deal with Android Size Alerts
2 2
3 *Most alerts should not have a bug created for them. Please read on...* 3 *Not all alerts should not have a bug created for them. Please read on...*
4 4
5 ### If the alert is for "other lib size" or "Unknown files size": 5 ### If the alert is for "other lib size" or "Unknown files size":
6 * File a bug against agrieve@ to fix 6 * File a bug against agrieve@ to fix
7 [resource_sizes.py](https://cs.chromium.org/chromium/src/build/android/resour ce_sizes.py). 7 [resource_sizes.py](https://cs.chromium.org/chromium/src/build/android/resour ce_sizes.py).
8 * ...or fix it yourself. This script will output the list of unknown 8 * ...or fix it yourself. This script will output the list of unknown
9 filenames. 9 filenames.
10 10
11 ### If the alert is a downstream size alert (aka, for Monochrome.apk): 11 ### If the alert is a downstream size alert (aka, for Monochrome.apk):
12 * The regression most likely already occurred in the upstream 12 * The regression most likely already occurred in the upstream
13 MonochromePublic.apk target. Look at the 13 MonochromePublic.apk target. Look at the
14 [upstream graphs](https://chromeperf.appspot.com/report?sid=cfc29eed1238fd38f b5e6cf83bdba6c619be621b606e03e5dfc2e99db14c418b&num_points=1500) 14 [upstream graphs](https://chromeperf.appspot.com/report?sid=cfc29eed1238fd38f b5e6cf83bdba6c619be621b606e03e5dfc2e99db14c418b&num_points=1500)
15 to find the culprit & de-dupe with upstream alerts. 15 to find the culprit & de-dupe with upstream alerts.
16 * If no upstream regression was found, look through the downstream commits 16 * If no upstream regression was found, look through the downstream commits
17 within the given date range to find the culprit. 17 within the given date range to find the culprit.
18 * Via `git log --format=fuller` (be sure to look at `CommitDate` and not 18 * Via `git log --format=fuller` (be sure to look at `CommitDate` and not
19 `AuthorDate`) 19 `AuthorDate`)
20 20
21 ### If the alert is for a roll: 21 ### If the alert is for a roll, or has multiple commits listed:
22 * Use a bisect to try and determine a more precise commit. 22 * Use a bisect to try and determine a more precise commit.
23 * Except don't. Bisects for these alerts [are currently broken](https://bugs .chromium.org/p/chromium/issues/detail?id=678338). 23 * Except don't. Bisects for these alerts [are currently broken](https://bugs .chromium.org/p/chromium/issues/detail?id=678338).
24 * Until this is fixed, just file a bug and assign to agrieve@.
24 25
25 ### What to do once the commit is identified: 26 ### What to do once the commit is identified:
26 * If the code seems to justify the size increase: 27 * If the code seems to justify the size increase:
27 1. Annotate the code review with the following (replacing **bold** parts): 28 1. Annotate the code review with the following (replacing **bold** parts):
28 > FYI - this added **20kb** to Chrome on Android. No action is required 29 > FYI - this added **20kb** to Chrome on Android. No action is required
29 > (unless you can think of an obvious way to reduce the overhead). 30 > (unless you can think of an obvious way to reduce the overhead).
30 > 31 >
31 > Link to size graph: 32 > Link to size graph:
32 [https://chromeperf.appspot.com/report?sid=cfc29eed1238fd38fb5e6cf83bdba6c619be6 21b606e03e5dfc2e99db14c418b&rev=**440074**](https://chromeperf.appspot.com/repor t?sid=cfc29eed1238fd38fb5e6cf83bdba6c619be621b606e03e5dfc2e99db14c418b&rev=44007 4) 33 [https://chromeperf.appspot.com/report?sid=cfc29eed1238fd38fb5e6cf83bdba6c619be6 21b606e03e5dfc2e99db14c418b&rev=**440074**](https://chromeperf.appspot.com/repor t?sid=cfc29eed1238fd38fb5e6cf83bdba6c619be621b606e03e5dfc2e99db14c418b&rev=44007 4)
33 2. Add an entry to 34 2. Add an entry to
34 [this spreadsheet](https://docs.google.com/spreadsheets/d/1GrRkszV7Oy5pVsa Mb5Eb6s8izW9t4dElBxIH3iGq93o/edit#gid=1894856744) 35 [this spreadsheet](https://docs.google.com/spreadsheets/d/1GrRkszV7Oy5pVsa Mb5Eb6s8izW9t4dElBxIH3iGq93o/edit#gid=1894856744)
35 to document the increase (also Update the "Themes / Thoughts" tab if 36 to document the increase (also Update the "Themes / Thoughts" tab if
36 applicable). 37 applicable).
37 * If the code might not justify the size increase: 38 * If the code might not justify the size increase:
38 1. File a bug and assign to the author to follow-up (and link them to 39 1. File a bug and assign to the author to follow-up (and link them to
39 [Debugging Apk Size Increase](https://chromium.googlesource.com/chromium/ src/+/master/tools/perf/docs/apk_size_regressions.md#Debugging-Apk-Size-Increase )). 40 [Debugging Apk Size Increase](https://chromium.googlesource.com/chromium/ src/+/master/tools/perf/docs/apk_size_regressions.md#Debugging-Apk-Size-Increase )).
41 * Change the bug's title from X% to XXkb
42 * Paste in link to commit that is at fault.
43 * Remove label: `Restrict-View-Google`
44 * Add label: `apk-size`
45 * TODO(agrieve): [https://github.com/catapult-project/catapult/issues/31 50](Change bug template to match these instructions)
40 2. Add an entry to 46 2. Add an entry to
41 [this spreadsheet](https://docs.google.com/spreadsheets/d/1GrRkszV7Oy5pVsa Mb5Eb6s8izW9t4dElBxIH3iGq93o/edit#gid=1894856744) 47 [this spreadsheet](https://docs.google.com/spreadsheets/d/1GrRkszV7Oy5pVsa Mb5Eb6s8izW9t4dElBxIH3iGq93o/edit#gid=1894856744)
42 to document the increase. 48 to document the increase.
43 49
44 # Debugging Apk Size Increase 50 # Debugging Apk Size Increase
45 51
46 ## Step 1: Identify where the extra bytes came from 52 ## Step 1: Identify where the extra bytes came from
47 53
48 Figure out which file within the .apk increased by looking at the size graphs 54 Figure out which file within the .apk increased by looking at the size graphs
49 showing the breakdowns. 55 showing the breakdowns.
(...skipping 30 matching lines...) Expand all
80 * This currently just shows a list of symbols added / removed rather tha n 86 * This currently just shows a list of symbols added / removed rather tha n
81 taking into account method body sizes. 87 taking into account method body sizes.
82 * Enhancements to this tool tracked at 88 * Enhancements to this tool tracked at
83 [crbug/678044](https://bugs.chromium.org/p/chromium/issues/detail?id=6 78044). 89 [crbug/678044](https://bugs.chromium.org/p/chromium/issues/detail?id=6 78044).
84 * If the growth is from images, ensure they are optimized: 90 * If the growth is from images, ensure they are optimized:
85 * Would it be smaller as a VectorDrawable? 91 * Would it be smaller as a VectorDrawable?
86 * If it's lossy, consider using webp. 92 * If it's lossy, consider using webp.
87 * Ensure you've optimized with 93 * Ensure you've optimized with
88 [tools/resources/optimize-png-files.sh](https://cs.chromium.org/chromium/s rc/tools/resources/optimize-png-files.sh). 94 [tools/resources/optimize-png-files.sh](https://cs.chromium.org/chromium/s rc/tools/resources/optimize-png-files.sh).
89 * There is some [Googler-specific guidance](https://goto.google.com/clank/en gineering/best-practices/adding-image-assets) as well. 95 * There is some [Googler-specific guidance](https://goto.google.com/clank/en gineering/best-practices/adding-image-assets) as well.
OLDNEW
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698