Improve post_build_ninja_summary.py's incremental build handling

post_build_ninja_summary.py detects a new incremental build by looking
for the end time stamps to go backwards. This means that repeated builds
that have a single long step will not be reliably detected and will, in
some cases, be missed entirely. This hits me sometimes when doing link
testing - delete an output DLL, rebuild, and the old results may be
displayed again.

This change updates the script to check for a different duration for an
existing record as well as an earlier end time should handle almost all
cases.

This also renames the targets local variable to avoid confusion due
to the targets member variable of the Targets class.

Bug: chromium:787983
Change-Id: I60a371df75d6cb7a55bd46b38156cb109feb8f15
Reviewed-on: https://chromium-review.googlesource.com/1061413
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>

post_build_ninja_summary.py

@@ -71,10 +71,10 @@ long_ext_count = 5
 class Target:
     """Represents a single line read for a .ninja_log file."""
     def __init__(self, start, end):
-        """Creates a target object by passing in the start/end times in ms."""
-        # Convert from milliseconds to seconds.
-        self.start = int(start) / 1000.0
-        self.end = int(end) / 1000.0
+        """Creates a target object by passing in the start/end times in seconds
+        as a float."""
+        self.start = start
+        self.end = end
         # A list of targets, appended to by the owner of this object.
         self.targets = []
         self.weighted_duration = 0.0
@@ -127,8 +127,8 @@ def ReadTargets(log, show_all):
     header = log.readline()
     assert header == '# ninja log v5\n', \
            'unrecognized ninja log version %r' % header
-    targets = {}
-    last_end_seen = 0
+    targets_dict = {}
+    last_end_seen = 0.0
     for line in log:
         parts = line.strip().split('\t')
         if len(parts) != 5:
@@ -136,14 +136,34 @@ def ReadTargets(log, show_all):
           # corrupt. Silently continue.
           continue
         start, end, _, name, cmdhash = parts # Ignore restat.
-        if not show_all and int(end) < last_end_seen:
+        # Convert from integral milliseconds to float seconds.
+        start = int(start) / 1000.0
+        end = int(end) / 1000.0
+        if not show_all and end < last_end_seen:
             # An earlier time stamp means that this step is the first in a new
             # build, possibly an incremental build. Throw away the previous
             # data so that this new build will be displayed independently.
-            targets = {}
-        last_end_seen = int(end)
-        targets.setdefault(cmdhash, Target(start, end)).targets.append(name)
-    return targets.values()
+            # This has to be done by comparing end times because records are
+            # written to the .ninja_log file when commands complete, so end
+            # times are guaranteed to be in order, but start times are not.
+            targets_dict = {}
+        target = None
+        if cmdhash in targets_dict:
+          target = targets_dict[cmdhash]
+          if not show_all and (target.start != start or target.end != end):
+            # If several builds in a row just run one or two build steps then
+            # the end times may not go backwards so the last build may not be
+            # detected as such. However in many cases there will be a build step
+            # repeated in the two builds and the changed start/stop points for
+            # that command, identified by the hash, can be used to detect and
+            # reset the target dictionary.
+            targets_dict = {}
+            target = None
+        if not target:
+          targets_dict[cmdhash] = target = Target(start, end)
+        last_end_seen = end
+        target.targets.append(name)
+    return targets_dict.values()
 
 
 def GetExtension(target):