New issue
Advanced search Search tips

Issue 710076 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 695182



Sign in to add a comment

Build a list of revs in diagnose_apk_size.py

Project Member Reported by agrieve@chromium.org, Apr 10 2017

Issue description

Needed when a roll happens, or when a size alert contains multiple commits.

I've been using a bash script for this, but it's too shameful to commit :P. Instead, I'll just paste it below.

The main learnings from having done this in the past:

1. Sanity check the number of commits is reasonable before starting
2. Abort if 3 builds in a row fail (this was more important when I was letting it run for weeks at a time)
3. Allow specifying sub-repo




#!/bin/bash

i=1
failed=0
BUILD_OUTPUT_DIR=out-gn/Release
SCRIPT_OUTPUT_DIR=out-gn/resource-sizes
CAUGHT_UP_HASH=04f54bf6
TARGET=chrome_public_apk
APK=ChromePublic.apk
LIB=lib.unstripped/libchrome.so
SUBREPO=v8
#04f54bf6..dc1b150e

if [[ -e $SCRIPT_OUTPUT_DIR ]]; then
  echo "Error: $SCRIPT_OUTPUT_DIR already exists."
  exit 1
fi

num_commits=$(cd $SUBREPO && git log $CAUGHT_UP_HASH^..HEAD --oneline | wc -l)
echo "$num_commits commits to build."

while true; do
  if [[ $SUBREPO = . ]]; then
    CHROME_HEADLESS=1 gclient sync
  fi
  rev=$(cd $SUBREPO && git rev-parse HEAD)
  echo "At rev: $rev"
  if [[ -z "$(cd $SUBREPO && git log $CAUGHT_UP_HASH^..HEAD -n1)" ]]; then
    echo "Caught up!"
    exit 0
  fi
  broken=
  if ! ( gn gen $BUILD_OUTPUT_DIR && cr-ninja.sh -C $BUILD_OUTPUT_DIR -j600 $TARGET ); then
    find $BUILD_OUTPUT_DIR -name "*.jar" -delete
    if ! ( cr-ninja.sh -C $BUILD_OUTPUT_DIR -j600 $TARGET ); then
      gn clean $BUILD_OUTPUT_DIR
      if ! ( gn gen $BUILD_OUTPUT_DIR && cr-ninja.sh -C $BUILD_OUTPUT_DIR -j600 $TARGET ); then
        broken=" broken"
      fi
    fi
  fi
  size="?"
  # mkdir after build so that it's not in the way if things fail early.
  mkdir -p $SCRIPT_OUTPUT_DIR
  if [[ -z "$broken" ]]; then
    out_base="$SCRIPT_OUTPUT_DIR/$i-$rev"
    cp $BUILD_OUTPUT_DIR/$LIB.map.gz $out_base.map.gz
    build/android/resource_sizes.py $BUILD_OUTPUT_DIR/apks/$APK --chartjson --no-output-dir > $out_base.txt
    mv results-chart.json $out_base.json
    size=$(python -c "import json; print json.load(open('$out_base.json'))['charts']['${APK}_Specifics']['normalized apk size']['value']")
    failed=0
  else
    failed=$(($failed + 1))
    if [[ $failed -gt 3 ]]; then
      echo 'Four failures in a row. Stopping.'
      exit 1
    fi
  fi
  filesize=$(stat --format %s $BUILD_OUTPUT_DIR/apks/$APK)
  echo "$i,$rev,$filesize,$size$broken" >> $SCRIPT_OUTPUT_DIR/all-normalized-sizes.txt
  i=$(($i + 1))
  (cd $SUBREPO && git checkout HEAD^)
done
 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 12 2017

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

commit 08f973975155004f8a9bc6887f6e9551cedbac1d
Author: estevenson <estevenson@chromium.org>
Date: Wed Apr 12 15:04:53 2017

Add resource_sizes diffs to diagnose_apk_bloat.py.

This CL adds the ability to show the differences in resource_sizes.py
metrics using dianose_apk_bloat.py. It also includes a few other
updates:
  * Simplfy output and write diff output to file
  * Generate and save .size files (and save .so files for Disassemble())
  * Be nicer by restoring user branches after script runs

BUG= 710076 

Review-Url: https://codereview.chromium.org/2810813003
Cr-Commit-Position: refs/heads/master@{#464023}

[modify] https://crrev.com/08f973975155004f8a9bc6887f6e9551cedbac1d/tools/binary_size/README.md
[modify] https://crrev.com/08f973975155004f8a9bc6887f6e9551cedbac1d/tools/binary_size/diagnose_apk_bloat.py

Project Member

Comment 2 by bugdroid1@chromium.org, Apr 21 2017

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

commit af790e8aa737385eb96d134ab18c475185671edc
Author: estevenson <estevenson@chromium.org>
Date: Fri Apr 21 21:45:25 2017

diagnose_apk_bloat.py: handle more rev options.

This CL introduces the ability to perform builds/diffs on all commits in
a specified range by passing the --all option. Other features added
include:
  * Perform local builds on commits in a subrepo (via --subrepo)
  * Store metadata and prevent unnecessary builds/downloads/diffs

BUG= 710076 

Review-Url: https://codereview.chromium.org/2834103002
Cr-Commit-Position: refs/heads/master@{#466456}

[modify] https://crrev.com/af790e8aa737385eb96d134ab18c475185671edc/tools/binary_size/README.md
[modify] https://crrev.com/af790e8aa737385eb96d134ab18c475185671edc/tools/binary_size/diagnose_apk_bloat.py

Blocking: 695182
Status: Fixed (was: Assigned)
Status: Started (was: Fixed)
Also thinking we should keep track of and log a summary of the largest offenders if you pass --all instead of having to manually check this (since with build output you often lose track of earlier output).
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 25 2017

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

commit 20184aeaa410802d38d8cad0d491ed9bed09d512
Author: estevenson <estevenson@chromium.org>
Date: Tue Apr 25 20:05:43 2017

diagnose_apk_bloat.py: add diff summary.

This CL changes diagnose_apk_bloat.py to write a summary for newly
generated diffs (normalized APK size delta for Android and native lib
delta for Linux).

BUG= 710076 

Review-Url: https://codereview.chromium.org/2837303002
Cr-Commit-Position: refs/heads/master@{#467083}

[modify] https://crrev.com/20184aeaa410802d38d8cad0d491ed9bed09d512/tools/binary_size/diagnose_apk_bloat.py

Status: Fixed (was: Started)
Labels: -binary-size Performance-Size

Sign in to add a comment