Split out optimization_advice.md from apk_size_regressions.md

I think there's enough content here for it to live on its own page.
This also adds more advice.

Change-Id: I84caac9fc80f92f8fcc7b3a3fbd07f45068425b9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1849095
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704337}

docs/speed/apk_size_regressions.md

@@ -106,74 +106,13 @@ for a description of binary size tools.**
 
 ## Step 2: Analyze
 
-### Growth is from Translations
-
- * There is likely nothing that can be done. Translations are expensive.
- * Close as `Won't Fix`.
-
-### Growth is from Native Resources (pak files)
-
- * Ensure `compress="gzip"` or `compress="brotli"` is used for all
-   highly-compressible (e.g. text) resources.
-   * Brotli compresses more but is much slower to decompress. Use brotli only
-     when performance doesn't matter much (e.g. internals pages).
- * Look at the SuperSize reports from the trybot to look for unexpected
-   resources, or unreasonably large symbols.
-
-### Growth is from Images
-
- * Would [a VectorDrawable](https://codereview.chromium.org/2857893003/) be better?
-   * If so, try optimizing it with [avocado](https://bugs.chromium.org/p/chromium/issues/detail?id=982302).
- * Would **lossy** compression make sense (often true for large images)?
-   * Then [use lossy webp](https://codereview.chromium.org/2615243002/).
-   * And omit some densities (e.g. add only an xxhdpi version).
- * Would **near-lossless** compression make sense (try it and see)?
-   * [Use pngquant](https://pngquant.org) to reduce the color depth without a
-     perceptual difference (use one of the GUI tools to compare before/afters).
- * Are the **lossless** images fully optimized?
-   * Use [tools/resources/optimize-png-files.sh](https://cs.chromium.org/chromium/src/tools/resources/optimize-png-files.sh).
-   * There is some [Googler-specific guidance](https://goto.google.com/clank/engineering/best-practices/adding-image-assets) as well.
-
-#### What Build-Time Image Optimizations are There?
- * For non-ninepatch images, `drawable-xxxhdpi` are omitted (they are not
-   perceptibly different from xxhdpi in most cases).
- * For non-ninepatch images within res/ directories (not for .pak file images),
-   they are converted to webp.
-   * Use the `android-binary-size` trybot to see the size of the images as webp,
-     or just build `ChromePublic.apk` and use `unzip -l` to see the size of the
-     images within the built apk.
-
-### Growth is from Native Code
-
- * Look at the SuperSize reports from the trybot to look for unexpected symbols,
-   or unreasonably large symbols.
- * If the diff looks reasonable, close as `Won't Fix`.
- * Otherwise, try to refactor a bit (e.g.
- [move code out of templates](https://bugs.chromium.org/p/chromium/issues/detail?id=716393)).
-   * Use [//tools/binary_size/diagnose_bloat.py](https://chromium.googlesource.com/chromium/src/+/master/tools/binary_size/README.md)
-     or the android-binary-size trybot to spot-check your local changes.
- * If symbols are larger than expected, use the `Disassemble()` feature of
-   `supersize console` to see what is going on.
-
-### Growth is from Java Code
-
- * Look at the SuperSize reports from the trybot to look for unexpected methods.
- * Ensure any new Java deps are as specific as possible.
-
-### Growth is from "other lib size" or "Unknown files size"
-
- * File a bug under [Tools > BinarySize](https://bugs.chromium.org/p/chromium/issues/list?q=component%3ATools%3EBinarySize)
-   with a link to your commit.
-
-### You Would Like Assistance
-
- * Feel free to email [binary-size@chromium.org](https://groups.google.com/a/chromium.org/forum/#!forum/binary-size).
+See [optimization advice](//docs/speed/binary_size/optimization_advice.md).
 
 ## Step 3: Give Up :/
 
 If you have spent O(days) trying to reduce the size overhead of your patch and
-are pretty sure that your implementation is efficient, then add a comment to the
-bug with the following:
+are pretty sure that your implementation is optimal(ish), then add a comment to
+the bug with the following:
 
 1) A description of where the size is coming from (show that you spent the time
    to understand why your code translated to a large binary size).

docs/speed/binary_size/optimization_advice.md

+# Optimizing Chrome's Binary Size
+
+ >
+ > This advice focuses on Android.
+ >
+
+[TOC]
+
+## How To Tell if It's Worth Spending Time on Binary Size?
+
+ * Binary size is a shared resource, and thus its growth is largely due to the
+   tragedy of the commons.
+ * It typically takes about a week of engineering time to reduce Android's
+   binary size by 50kb.
+ * As of 2019, Chrome for Android (arm32) grows by about 100kb per week.
+
+## Optimizing Translations (Strings)
+
+ * Use [Android System strings](https://developer.android.com/reference/android/R.string.html) where appropriate
+ * Ensure that strings in .grd files need to be there. For strings that do
+   not need to be translated, put them directly in source code.
+
+## Optimizing Non-Image Native Resources in .pak Files
+
+ * Ensure `compress="gzip"` or `compress="brotli"` is used for all
+   highly-compressible (e.g. text) resources.
+   * Brotli compresses more but is much slower to decompress. Use brotli only
+     when performance doesn't matter (e.g. internals pages).
+ * Look at the SuperSize reports from the android-binary-size trybot to look for
+   unexpected resources, or unreasonably large symbols.
+
+## Optimizing Images
+
+ * Would a vector image work?
+   * For images used in native code: [VectorIcon](https://chromium.googlesource.com/chromium/src/+/HEAD/components/vector_icons/README.md).
+   * For Android drawables: [VectorDrawable](https://codereview.chromium.org/2857893003/).
+     * Optimize vector drawables with [avocado](https://bugs.chromium.org/p/chromium/issues/detail?id=982302).
+ * Would **lossy** compression make sense (often true for large images)?
+   * If so, [use lossy webp](https://codereview.chromium.org/2615243002/).
+   * And omit some densities (e.g. add only an xxhdpi version).
+ * Would **near-lossless** compression make sense?
+   * This can often reduce size by >50% without a perceptible difference.
+   * [Use pngquant](https://pngquant.org) to try this out (use one of the GUI
+     tools to compare before/after).
+ * Are the **lossless** images fully optimized?
+   * Use [tools/resources/optimize-png-files.sh](https://cs.chromium.org/chromium/src/tools/resources/optimize-png-files.sh).
+   * There is some [Googler-specific guidance](https://goto.google.com/clank/engineering/best-practices/adding-image-assets) as well.
+
+### What Build-Time Image Optimizations are There?
+ * For non-ninepatch images, `drawable-xxxhdpi` are omitted (they are not
+   perceptibly different from xxhdpi in most cases).
+ * For non-ninepatch images within res/ directories (not for .pak file images),
+   they are converted to webp.
+   * Use the `android-binary-size` trybot to see the size of the images as webp,
+     or just build `ChromePublic.apk` and use `unzip -l` to see the size of the
+     images within the built apk.
+
+## Optimizing Code
+
+In most parts of the codebase, you should try to optimize your code for binary
+size rather than performance. Most code runs "fast enough" and only needs to be
+performance-optimized if identified as a hot spot. Individual code size affects
+overall binary size no matter the utility of the code.
+
+What this *could* mean in practice?
+ * Use a linear search over an array rather than a binary search over a sorted
+   one.
+ * Reuse common code rather than writing optimized purpose-specific code.
+
+Practical advice:
+ * When making changes, look at symbol breakdowns with SuperSize reports from
+   the [android-binary-size trybot][size-trybot].
+   * Or use [//tools/binary_size/diagnose_bloat.py][diagnose_bloat] to create
+     diffs locally.
+ * Ensure no symbols exists that are used only by tests.
+ * Be concise with strings used for error handling.
+   * Identical strings throughout the codebase are de-duped. Take advantage of
+     this for error-related strings.
+ * If there's a notable increase in `.data.rel.ro`:
+   * Ensure there are not [excessive relocations][relocations].
+ * If there's a notable increase in `.rodata`:
+   * See if it would make sense to compress large symbols here by moving them to
+     .pak files.
+   * Gut-check that all unique string literals being added are actually useful.
+ * If there's a notable increase in `.text`:
+   * If there are a lot of symbols from C++ templates, try moving functions
+     that don't use template parameters to
+     [non-templated helper functions][template_bloat]).
+     * And extract parts of functions that don't use them into helper functions.
+   * Try to leverage identical-code-folding as much as possible by making the
+     shape of your code consistent.
+     * E.g. Use PODs wherever possible, and especially in containers. They will
+       likely compile down to the same code as other pre-existing PODs.
+       * Try also to use consistent field ordering within PODs.
+     * E.g. a `std::vector` of bare pointers will very likely by ICF'ed, but one
+       that uses smart pointers gets type-specific destructor logic inlined into
+       it.
+     * This advice is especially applicable to generated code.
+   * If symbols are larger than expected, use the `Disassemble()` feature of
+     [`supersize console`][supersize-console] to see what is going on.
+     * Watch out for inlined constructors & destructors. E.g. having parameters
+       that are passed by value forces callers to construct objects before
+       calling.
+       * E.g. For frequently called functions, it can make sense to provide
+         separate `const char *` and `const std::string&` overloads rather than
+         a single `base::StringPiece`.
+
+Android-specific advice:
+ * Prefer fewer large JNI calls over many small JNI calls.
+ * Minimize the use of class initializers (`<clinit>()`).
+   * If R8 cannot determine that they are "trivial", they will prevent
+     inlining of static members on the class.
+   * In C++, static objects are created at compile time, but in Java they
+     are created by executing code within `<clinit>()`. There is often little
+     advantage to initializing class fields statically vs. upon first use.
+ * Don't use default interface methods on interfaces with multiple implementers.
+   * Desugaring causes the methods to be added to every implementor separately.
+   * It's more efficient to use a base class to add default methods.
+ * Use config-specific resource directories sparingly.
+   * Introducing a new config has [a large cost][arsc-bloat].
+ * Use `String.format()` instead of concatenation.
+   * Concatenation causes a lot of StringBuilder code to be generated.
+ * Try to use default values for fields rather than explicit initialization.
+   * E.g. Name booleans such that they start as "false".
+   * E.g. Use integer sentinels that have initial state as 0.
+ * Minimize the number of callbacks / lambdas that each API requires.
+   * Each callback / lambda is syntactic sugar for an anonymous class, and all
+     classes have a constructor in addition to the callback method.
+   * E.g. rather than have `onFailure()` vs `onSuccess()`, have an
+     `onFinished(bool)`.
+   * E.g. rather than have `onTextChanged()`, `onDateChanged()`, ..., have a
+     single `onChanged()` that assumes everything changed.
+
+[size-trybot]: //tools/binary_size/README.md#Binary-Size-Trybot-android_binary_size
+[diagnose_bloat]: //tools/binary_size/README.md#diagnose_bloat_py
+[relocations]: //docs/native_relocations.md
+[template_bloat]: https://bugs.chromium.org/p/chromium/issues/detail?id=716393
+[supersize-console]: //tools/binary_size/README.md#Usage_console
+[arsc-bloat]: https://medium.com/androiddevelopers/smallerapk-part-3-removing-unused-resources-1511f9e3f761#0b72
+
+## Optimizing Third-Party Android Dependencies
+
+ * Look through SuperSize symbols to see whether unwanted functionality
+   is being pulled in.
+   * Use ProGuard's `-whyareyoukeeping` to see why unwanted symbols are kept.
+   * Try adding `-assumenosideeffects` rules to strip out unwanted calls.
+ * Consider removing all resources via `strip_resources = true`.
+ * Remove specific drawables via `resource_blacklist_regex`.
+
+## Size Optimization Help
+
+ * Feel free to email [binary-size@chromium.org](https://groups.google.com/a/chromium.org/forum/#!forum/binary-size).

tools/binary_size/README.md

@@ -38,9 +38,9 @@ Per-Milestone Binary Size Breakdowns:
  * Shows you only the binary size metrics your patchset affects.
  * Links to SuperSize html and text output for more details on which symbols
    were changed.
- * For tips on reducing binary size, see [Debugging Apk Size Increase][debugging-apk-size-increase].
+ * For tips on reducing binary size, see [Optimization Advice][optimization_advice].
 
-[debugging-apk-size-increase]: https://chromium.googlesource.com/chromium/src/+/HEAD/docs/speed/apk_size_regressions.md#debugging-apk-size-increase
+[optimization_advice]: //docs/speed/binary_size/optimization_advice.md
 
 ## resource_sizes.py
 
@@ -50,7 +50,7 @@ Per-Milestone Binary Size Breakdowns:
    [chromeperf](https://chromeperf.appspot.com/report) under
    `Test suite="resource_sizes ($APK)"`.
  * Metrics reported by this tool are described in
-   [//docs/speed/binary_size/metrics.md](../../docs/speed/binary_size/metrics.md).
+   [//docs/speed/binary_size/metrics.md](//docs/speed/binary_size/metrics.md).
 
 ## SuperSize
 

tools/binary_size/trybot_commit_size_checker.py

@@ -34,7 +34,7 @@ be mitigated.
 
 There is guidance at:
 
-https://chromium.googlesource.com/chromium/src/+/master/docs/speed/apk_size_regressions.md#Debugging-Apk-Size-Increase
+https://chromium.googlesource.com/chromium/src/+/master/docs/speed/apk_size_regressions.md
 
 If the growth is expected / justified, then you can bypass this bot failure by
 adding "Binary-Size: $JUSTIFICATION" footer to your commit message (must go at