Commit eb74ff2f authored by Samuel Huang's avatar Samuel Huang Committed by Commit Bot

[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/1246280Reviewed-by: default avataragrieve <agrieve@chromium.org>
Reviewed-by: default avatarSamuel Huang <huangs@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594431}
parent d6dce47d
...@@ -112,6 +112,9 @@ def _MakeTreeViewList(symbols, include_all_symbols): ...@@ -112,6 +112,9 @@ def _MakeTreeViewList(symbols, include_all_symbols):
small_symbol_pss = collections.defaultdict( small_symbol_pss = collections.defaultdict(
lambda: collections.defaultdict(float)) lambda: collections.defaultdict(float))
# For delta symbols, most counts should 0, so use that as default. Else use 1.
default_symbol_count = 0 if symbols.IsDelta() else 1
if include_all_symbols: if include_all_symbols:
main_symbols, extra_symbols = symbols, [] main_symbols, extra_symbols = symbols, []
else: else:
...@@ -124,9 +127,9 @@ def _MakeTreeViewList(symbols, include_all_symbols): ...@@ -124,9 +127,9 @@ def _MakeTreeViewList(symbols, include_all_symbols):
symbol_size = round(symbol.pss, 2) symbol_size = round(symbol.pss, 2)
if symbol_size.is_integer(): if symbol_size.is_integer():
symbol_size = int(symbol_size) symbol_size = int(symbol_size)
symbol_count = 1 symbol_count = default_symbol_count
if symbol.IsDelta() and symbol.diff_status == models.DIFF_STATUS_REMOVED: if symbol.IsDelta():
symbol_count = -1 symbol_count = models.DIFF_COUNT_DELTA[symbol.diff_status]
path = symbol.source_path or symbol.object_path path = symbol.source_path or symbol.object_path
file_node = _GetOrAddFileNode( file_node = _GetOrAddFileNode(
...@@ -146,7 +149,7 @@ def _MakeTreeViewList(symbols, include_all_symbols): ...@@ -146,7 +149,7 @@ def _MakeTreeViewList(symbols, include_all_symbols):
# count as -1 rather than the default, 1. # count as -1 rather than the default, 1.
# We don't care about accurate counts for other symbol types currently, # We don't care about accurate counts for other symbol types currently,
# so this data is only included for methods. # so this data is only included for methods.
if is_dex_method and symbol_count != 1: if is_dex_method and symbol_count != default_symbol_count:
symbol_entry[_COMPACT_SYMBOL_COUNT_KEY] = symbol_count symbol_entry[_COMPACT_SYMBOL_COUNT_KEY] = symbol_count
if symbol.flags: if symbol.flags:
symbol_entry[_COMPACT_SYMBOL_FLAGS_KEY] = symbol.flags symbol_entry[_COMPACT_SYMBOL_FLAGS_KEY] = symbol.flags
......
...@@ -152,6 +152,7 @@ DIFF_STATUS_CHANGED = 1 ...@@ -152,6 +152,7 @@ DIFF_STATUS_CHANGED = 1
DIFF_STATUS_ADDED = 2 DIFF_STATUS_ADDED = 2
DIFF_STATUS_REMOVED = 3 DIFF_STATUS_REMOVED = 3
DIFF_PREFIX_BY_STATUS = ['= ', '~ ', '+ ', '- '] DIFF_PREFIX_BY_STATUS = ['= ', '~ ', '+ ', '- ']
DIFF_COUNT_DELTA = [0, 0, 1, -1]
STRING_LITERAL_NAME = 'string literal' STRING_LITERAL_NAME = 'string literal'
......
...@@ -361,8 +361,9 @@ class TreeBuilder { ...@@ -361,8 +361,9 @@ class TreeBuilder {
* tree nodes for that file's symbols attached. Afterwards attach that node to * tree nodes for that file's symbols attached. Afterwards attach that node to
* its parent directory node, or create it if missing. * its parent directory node, or create it if missing.
* @param {FileEntry} fileEntry File entry from data file * @param {FileEntry} fileEntry File entry from data file
* @param {boolean} diffMode Whether diff mode is in effect.
*/ */
addFileEntry(fileEntry) { addFileEntry(fileEntry, diffMode) {
const idPath = this._getPath(fileEntry); const idPath = this._getPath(fileEntry);
// make node for this // make node for this
const fileNode = createNode({ const fileNode = createNode({
...@@ -370,13 +371,15 @@ class TreeBuilder { ...@@ -370,13 +371,15 @@ class TreeBuilder {
shortNameIndex: lastIndexOf(idPath, this._sep) + 1, shortNameIndex: lastIndexOf(idPath, this._sep) + 1,
type: _CONTAINER_TYPES.FILE, type: _CONTAINER_TYPES.FILE,
}); });
const defaultCount = diffMode ? 0 : 1;
// build child nodes for this file's symbols and attach to self // build child nodes for this file's symbols and attach to self
for (const symbol of fileEntry[_KEYS.FILE_SYMBOLS]) { for (const symbol of fileEntry[_KEYS.FILE_SYMBOLS]) {
const size = symbol[_KEYS.SIZE]; const size = symbol[_KEYS.SIZE];
const type = symbol[_KEYS.TYPE]; const type = symbol[_KEYS.TYPE];
const count = symbol[_KEYS.COUNT] || 1; const count = _KEYS.COUNT in symbol ? symbol[_KEYS.COUNT] : defaultCount;
const flags = symbol[_KEYS.FLAGS] || 0; const flags = _KEYS.FLAGS in symbol ? symbol[_KEYS.FLAGS] : 0;
const numAliases = symbol[_KEYS.NUM_ALIASES] || 1; const numAliases =
_KEYS.NUM_ALIASES in symbol ? symbol[_KEYS.NUM_ALIASES] : 1;
const symbolNode = createNode({ const symbolNode = createNode({
// Join file path to symbol name with a ":" // Join file path to symbol name with a ":"
...@@ -758,13 +761,15 @@ async function buildTree(groupBy, filterTest, highlightTest, onProgress) { ...@@ -758,13 +761,15 @@ async function buildTree(groupBy, filterTest, highlightTest, onProgress) {
try { try {
// Post partial state every second // Post partial state every second
let lastBatchSent = Date.now(); let lastBatchSent = Date.now();
let diffMode = null;
for await (const dataObj of fetcher.newlineDelimtedJsonStream()) { for await (const dataObj of fetcher.newlineDelimtedJsonStream()) {
if (meta == null) { if (meta == null) {
// First line of data is used to store meta information. // First line of data is used to store meta information.
meta = /** @type {Meta} */ (dataObj); meta = /** @type {Meta} */ (dataObj);
diffMode = meta.diff_mode;
postToUi(); postToUi();
} else { } else {
builder.addFileEntry(/** @type {FileEntry} */ (dataObj)); builder.addFileEntry(/** @type {FileEntry} */ (dataObj), diffMode);
const currentTime = Date.now(); const currentTime = Date.now();
if (currentTime - lastBatchSent > 500) { if (currentTime - lastBatchSent > 500) {
postToUi(); postToUi();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment