summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Tao Bao <tbao@google.com> 2018-02-08 23:21:52 -0800
committer Tao Bao <tbao@google.com> 2018-02-09 23:02:32 -0800
commit294651d7b4e56559822894ccc520f194363d10d8 (patch)
tree90fd080d61700b11242b68d0a0653ff6f7d83822
parentbec8be51a5cbc6e9a14daec2bf92a47fdb262366 (diff)
releasetools: Add an ImgdiffStats class that reports imgdiff stats.
We have a couple of active imgdiff workarounds (and likely with one more inbounding that allows having shared blocks in ext4 image). Most of these workarounds need extending imgdiff's capability. While us not getting there anytime soon, collect the stats to better understand the impact of each kind so we can prioritize accordingly. A sample report is as follows. Imgdiff Stats Report ======================== APK files diff'd with imgdiff (count: 88) ------------------------------------------- /system/priv-app/Shell/Shell.apk ... Large APK files split and diff'd with imgdiff (count: 4) ---------------------------------------------------------- /system/priv-app/Settings/Settings.apk ... Bug: 68016761 Test: Generate an incremental BBOTA package. Check the stats report. Test: python -m unittest test_blockimgdiff Change-Id: I27ad862cde472ab2806db877632ce5a0607420f2
-rw-r--r--tools/releasetools/blockimgdiff.py97
-rw-r--r--tools/releasetools/test_blockimgdiff.py43
2 files changed, 130 insertions, 10 deletions
diff --git a/tools/releasetools/blockimgdiff.py b/tools/releasetools/blockimgdiff.py
index f1cb6dba88..fe37b391da 100644
--- a/tools/releasetools/blockimgdiff.py
+++ b/tools/releasetools/blockimgdiff.py
@@ -256,6 +256,71 @@ class HeapItem(object):
return self.score <= other.score
+class ImgdiffStats(object):
+ """A class that collects imgdiff stats.
+
+ It keeps track of the files that will be applied imgdiff while generating
+ BlockImageDiff. It also logs the ones that cannot use imgdiff, with specific
+ reasons. The stats is only meaningful when imgdiff not being disabled by the
+ caller of BlockImageDiff. In addition, only files with supported types
+ (BlockImageDiff.FileTypeSupportedByImgdiff()) are allowed to be logged.
+
+ TODO: The info could be inaccurate due to the unconditional fallback from
+ imgdiff to bsdiff on errors. The fallbacks will be removed.
+ """
+
+ USED_IMGDIFF = "APK files diff'd with imgdiff"
+ USED_IMGDIFF_LARGE_APK = "Large APK files split and diff'd with imgdiff"
+
+ # Reasons for not applying imgdiff on APKs.
+ SKIPPED_TRIMMED = "Not used imgdiff due to trimmed RangeSet"
+ SKIPPED_NONMONOTONIC = "Not used imgdiff due to having non-monotonic ranges"
+
+ # The list of valid reasons, which will also be the dumped order in a report.
+ REASONS = (
+ USED_IMGDIFF,
+ USED_IMGDIFF_LARGE_APK,
+ SKIPPED_TRIMMED,
+ SKIPPED_NONMONOTONIC,
+ )
+
+ def __init__(self):
+ self.stats = {}
+
+ def Log(self, filename, reason):
+ """Logs why imgdiff can or cannot be applied to the given filename.
+
+ Args:
+ filename: The filename string.
+ reason: One of the reason constants listed in REASONS.
+
+ Raises:
+ AssertionError: On unsupported filetypes or invalid reason.
+ """
+ assert BlockImageDiff.FileTypeSupportedByImgdiff(filename)
+ assert reason in self.REASONS
+
+ if reason not in self.stats:
+ self.stats[reason] = set()
+ self.stats[reason].add(filename)
+
+ def Report(self):
+ """Prints a report of the collected imgdiff stats."""
+
+ def print_header(header, separator):
+ print(header)
+ print(separator * len(header) + '\n')
+
+ print_header(' Imgdiff Stats Report ', '=')
+ for key in self.REASONS:
+ if key not in self.stats:
+ continue
+ values = self.stats[key]
+ section_header = ' {} (count: {}) '.format(key, len(values))
+ print_header(section_header, '-')
+ print(''.join([' {}\n'.format(name) for name in values]))
+
+
# BlockImageDiff works on two image objects. An image object is
# anything that provides the following attributes:
#
@@ -311,6 +376,7 @@ class BlockImageDiff(object):
self.touched_src_ranges = RangeSet()
self.touched_src_sha1 = None
self.disable_imgdiff = disable_imgdiff
+ self.imgdiff_stats = ImgdiffStats() if not disable_imgdiff else None
assert version in (3, 4)
@@ -337,7 +403,7 @@ class BlockImageDiff(object):
"""Returns whether the file type is supported by imgdiff."""
return filename.lower().endswith(('.apk', '.jar', '.zip'))
- def CanUseImgdiff(self, name, tgt_ranges, src_ranges):
+ def CanUseImgdiff(self, name, tgt_ranges, src_ranges, large_apk=False):
"""Checks whether we can apply imgdiff for the given RangeSets.
For files in ZIP format (e.g., APKs, JARs, etc.) we would like to use
@@ -359,17 +425,26 @@ class BlockImageDiff(object):
name: The filename to be diff'd.
tgt_ranges: The target RangeSet.
src_ranges: The source RangeSet.
+ large_apk: Whether this is to split a large APK.
Returns:
A boolean result.
"""
- return (
- not self.disable_imgdiff and
- self.FileTypeSupportedByImgdiff(name) and
- tgt_ranges.monotonic and
- src_ranges.monotonic and
- not tgt_ranges.extra.get('trimmed') and
- not src_ranges.extra.get('trimmed'))
+ if (self.disable_imgdiff or not self.FileTypeSupportedByImgdiff(name)):
+ return False
+
+ if not tgt_ranges.monotonic or not src_ranges.monotonic:
+ self.imgdiff_stats.Log(name, ImgdiffStats.SKIPPED_NONMONOTONIC)
+ return False
+
+ if tgt_ranges.extra.get('trimmed') or src_ranges.extra.get('trimmed'):
+ self.imgdiff_stats.Log(name, ImgdiffStats.SKIPPED_TRIMMED)
+ return False
+
+ reason = (ImgdiffStats.USED_IMGDIFF_LARGE_APK if large_apk
+ else ImgdiffStats.USED_IMGDIFF)
+ self.imgdiff_stats.Log(name, reason)
+ return True
def Compute(self, prefix):
# When looking for a source file to use as the diff input for a
@@ -404,6 +479,10 @@ class BlockImageDiff(object):
self.ComputePatches(prefix)
self.WriteTransfers(prefix)
+ # Report the imgdiff stats.
+ if common.OPTIONS.verbose and not self.disable_imgdiff:
+ self.imgdiff_stats.Report()
+
def WriteTransfers(self, prefix):
def WriteSplitTransfers(out, style, target_blocks):
"""Limit the size of operand in command 'new' and 'zero' to 1024 blocks.
@@ -1289,7 +1368,7 @@ class BlockImageDiff(object):
# calling the costly RangeSha1()s.
if (self.FileTypeSupportedByImgdiff(tgt_name) and
self.tgt.RangeSha1(tgt_ranges) != self.src.RangeSha1(src_ranges)):
- if self.CanUseImgdiff(tgt_name, tgt_ranges, src_ranges):
+ if self.CanUseImgdiff(tgt_name, tgt_ranges, src_ranges, True):
large_apks.append((tgt_name, src_name, tgt_ranges, src_ranges))
return
diff --git a/tools/releasetools/test_blockimgdiff.py b/tools/releasetools/test_blockimgdiff.py
index 4ed8356dd2..a2552d65ac 100644
--- a/tools/releasetools/test_blockimgdiff.py
+++ b/tools/releasetools/test_blockimgdiff.py
@@ -19,7 +19,8 @@ from __future__ import print_function
import unittest
import common
-from blockimgdiff import BlockImageDiff, EmptyImage, HeapItem, Transfer
+from blockimgdiff import (BlockImageDiff, EmptyImage, HeapItem, ImgdiffStats,
+ Transfer)
from rangelib import RangeSet
@@ -196,6 +197,17 @@ class BlockImageDiffTest(unittest.TestCase):
self.assertTrue(
block_image_diff.CanUseImgdiff(
"/system/app/app1.apk", RangeSet("10-15"), RangeSet("0-5")))
+ self.assertTrue(
+ block_image_diff.CanUseImgdiff(
+ "/vendor/app/app2.apk", RangeSet("20 25"), RangeSet("30-31"), True))
+
+ self.assertDictEqual(
+ {
+ ImgdiffStats.USED_IMGDIFF : {"/system/app/app1.apk"},
+ ImgdiffStats.USED_IMGDIFF_LARGE_APK : {"/vendor/app/app2.apk"},
+ },
+ block_image_diff.imgdiff_stats.stats)
+
def test_CanUseImgdiff_ineligible(self):
# Disabled by caller.
@@ -223,3 +235,32 @@ class BlockImageDiffTest(unittest.TestCase):
self.assertFalse(
block_image_diff.CanUseImgdiff(
"/vendor/app/app3.apk", RangeSet("10-15"), src_ranges))
+
+ # The stats are correctly logged.
+ self.assertDictEqual(
+ {
+ ImgdiffStats.SKIPPED_NONMONOTONIC : {'/system/app/app2.apk'},
+ ImgdiffStats.SKIPPED_TRIMMED : {'/vendor/app/app3.apk'},
+ },
+ block_image_diff.imgdiff_stats.stats)
+
+
+class ImgdiffStatsTest(unittest.TestCase):
+
+ def test_Log(self):
+ imgdiff_stats = ImgdiffStats()
+ imgdiff_stats.Log("/system/app/app2.apk", ImgdiffStats.USED_IMGDIFF)
+ self.assertDictEqual(
+ {
+ ImgdiffStats.USED_IMGDIFF: {'/system/app/app2.apk'},
+ },
+ imgdiff_stats.stats)
+
+ def test_Log_invalidInputs(self):
+ imgdiff_stats = ImgdiffStats()
+
+ self.assertRaises(AssertionError, imgdiff_stats.Log, "/system/bin/gzip",
+ ImgdiffStats.USED_IMGDIFF)
+
+ self.assertRaises(AssertionError, imgdiff_stats.Log, "/system/app/app1.apk",
+ "invalid reason")