New issue
Advanced search Search tips

Issue 884985 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 28
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Tiger Viewer Method Count Mode Wrong for Milestones

Project Member Reported by agrieve@chromium.org, Sep 18

Issue description

This bug arises from 2 issues:

(1) html_report._MakeTreeViewList(): By default symbol_count is 1, and is assigned -1 if symbol is removed. But for diff mode, if symbol is changed then (delta) symbol count should be 0.

(2) tree_worker.js: TreeBuilder.addFileEntry(): The line
  const count = symbol[_KEYS.COUNT] || 1;
Force 0 (and undefined) to 1.

This is not too hard to fix. However, to optimize for .ndjson size, we'd also like to set symbol_count default to 1 for single-binary .ndjson, and 0 for .ndjson for binary-diff .ndjson.

Project Member

Comment 2 by bugdroid1@chromium.org, Sep 26

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

commit eb74ff2fcc974617103437fc5ad7ea2ec6f8448b
Author: Samuel Huang <huangs@chromium.org>
Date: Wed Sep 26 19:31:59 2018

[SuperSize] HTML Viewer: Fix Method Count under diff mode.

Under diff mode, previously the HTML Viewer would report very large
method (delta) counts. This was caused by the Python code assigning a
delta count of 1 (instead of 0) for changed symbols. Moreover, to
support delta count of 0, the JavaScript code change needs to be
updated to read it (i.e., cannot use "||" to assign defaults).

This CL fixes the above problems. Moreover, to reduce .ndjson file
size, 0 is used as the default symbol count under diff mode (1 is still
the defalut if one .size file is used to generate the .ndjson file).

Bug:  884985 
Change-Id: I8038f8b8298073c8ced5ccf9100cf9911e533861
Reviewed-on: https://chromium-review.googlesource.com/1246280
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594431}
[modify] https://crrev.com/eb74ff2fcc974617103437fc5ad7ea2ec6f8448b/tools/binary_size/libsupersize/html_report.py
[modify] https://crrev.com/eb74ff2fcc974617103437fc5ad7ea2ec6f8448b/tools/binary_size/libsupersize/models.py
[modify] https://crrev.com/eb74ff2fcc974617103437fc5ad7ea2ec6f8448b/tools/binary_size/libsupersize/static/tree-worker.js

I ran tools/binary_size/libsupersize/generate_milestone_report.py and updated all the reports but it's still showing 33k methods for me at https://storage.googleapis.com/chrome-supersize/index.html?data_url=milestones%2Farm%2FMonochrome.apk%2Freport_68.0.3440.85_69.0.3497.91.ndjson&method_count=on&diff_mode=on.

Then I downloaded the report locally (gsutil cp gs://chrome-supersize/milestones/arm/Monochrome.apk/report_68.0.3440.85_69.0.3497.91.ndjson .
) and it worked  
Status: Fixed (was: Assigned)
Oops, I had forgotten to run tools/binary_size/libsupersize/upload_html_viewer.py.

There seems to be a caching problem though since I can't seem to receive the update template when loading in Chrome (requires incognito).

Sign in to add a comment