Improve parsing error handling in dmprof.

tools/deep_memory_profiler/dmprof

Index: tools/deep_memory_profiler/dmprof
diff --git a/tools/deep_memory_profiler/dmprof b/tools/deep_memory_profiler/dmprof
index fdf5a7c414ead3853c2f1ba29f649819603bcb06..e6bc93ecb5a03e562f49ec1fdd5e21ce3caad9a7 100755
--- a/tools/deep_memory_profiler/dmprof
+++ b/tools/deep_memory_profiler/dmprof
@@ -66,6 +66,13 @@ appeared_addresses = set()
 components = []
 
 
+class ParsingException(Exception):
+  def __init__(self, value):
+    self.value = value
+  def __str__(self):
+    return repr(self.value)
+
+
 class Policy(object):
 
   def __init__(self, name, mmap, pattern):
@@ -112,17 +119,16 @@ class Bucket(object):
 class Log(object):
 
   """A class representing one dumped log data."""
-  def __init__(self, log_path, buckets):
+  def __init__(self, log_path):
     self.log_path = log_path
     self.log_lines = [
         l for l in open(self.log_path, 'r') if l and not l.startswith('#')]
     self.log_version = ''
-    sys.stderr.write('parsing a log file:%s\n' % log_path)
+    sys.stderr.write('Loading a dump: %s\n' % log_path)
     self.mmap_stacktrace_lines = []
     self.malloc_stacktrace_lines = []
     self.counters = {}
     self.log_time = os.stat(self.log_path).st_mtime
-    self.parse_log(buckets)
 
   @staticmethod
   def dump_stacktrace_lines(stacktrace_lines, buckets):
@@ -279,13 +285,17 @@ class Log(object):
   @staticmethod
   def skip_lines_while(line_number, max_line_number, skipping_condition):
     """Increments line_number until skipping_condition(line_number) is false.
+
+    Returns:
+        A pair of an integer indicating a line number after skipped, and a
+        boolean value which is True if found a line which skipping_condition
+        is False for.
     """
     while skipping_condition(line_number):
       line_number += 1
       if line_number >= max_line_number:
-        sys.stderr.write('invalid heap profile dump.')
-        return line_number
-    return line_number
+        return line_number, False
+    return line_number, True
 
   def parse_stacktraces_while_valid(self, buckets, log_lines, line_number):
     """Parses stacktrace lines while the lines are valid.
@@ -301,11 +311,11 @@ class Log(object):
         A pair of a list of valid lines and an integer representing the last
         line number in log_lines.
     """
-    line_number = self.skip_lines_while(
+    (line_number, found) = self.skip_lines_while(

[M-A Ruel, 2012/04/16 12:20:55]

since found is not used, you can either:
line_number = self.skip_lines_while(...)[0]
or
(lin_number, _) = self.skip_lines_while(...)

[Dai Mikurube (NOT FULLTIME), 2012/04/17 03:26:32]

Done.

         line_number, len(log_lines),
         lambda n: not log_lines[n].split()[0].isdigit())
     stacktrace_lines_start = line_number
-    line_number = self.skip_lines_while(
+    (line_number, found) = self.skip_lines_while(
         line_number, len(log_lines),
         lambda n: self.check_stacktrace_line(log_lines[n], buckets))
     return (log_lines[stacktrace_lines_start:line_number], line_number)
@@ -323,15 +333,15 @@ class Log(object):
             log_lines.
 
     Raises:
-        RuntimeException for invalid dump versions.
+        ParsingException for invalid dump versions.
     """
-    sys.stderr.write('  heap profile dump version: %s\n' % self.log_version)
+    sys.stderr.write('  Version: %s\n' % self.log_version)
 
     if self.log_version in (DUMP_DEEP_3, DUMP_DEEP_4):
       (self.mmap_stacktrace_lines, line_number) = (
           self.parse_stacktraces_while_valid(
               buckets, self.log_lines, line_number))
-      line_number = self.skip_lines_while(
+      (line_number, found) = self.skip_lines_while(
           line_number, len(self.log_lines),
           lambda n: self.log_lines[n] != 'MALLOC_STACKTRACES:\n')
       (self.malloc_stacktrace_lines, line_number) = (
@@ -342,7 +352,7 @@ class Log(object):
       (self.mmap_stacktrace_lines, line_number) = (
           self.parse_stacktraces_while_valid(
               buckets, self.log_lines, line_number))
-      line_number = self.skip_lines_while(
+      (line_number, found) = self.skip_lines_while(
           line_number, len(self.log_lines),
           lambda n: self.log_lines[n] != 'MALLOC_STACKTRACES:\n')
       (self.malloc_stacktrace_lines, line_number) = (
@@ -357,12 +367,12 @@ class Log(object):
               buckets, self.log_lines, line_number))
 
     else:
-      raise RuntimeError('invalid heap profile dump version: %s' % (
+      raise ParsingException('invalid heap profile dump version: %s' % (
           self.log_version))
 
   def parse_global_stats(self):
     """Parses lines in self.log_lines as global stats."""
-    ln = self.skip_lines_while(
+    (ln, found) = self.skip_lines_while(
         0, len(self.log_lines),
         lambda n: self.log_lines[n] != 'GLOBAL_STATS:\n')
 
@@ -378,7 +388,7 @@ class Log(object):
           'total', 'file', 'anonymous', 'other', 'mmap', 'tcmalloc']
 
     for prefix in global_stat_names:
-      ln = self.skip_lines_while(
+      (ln, found) = self.skip_lines_while(
           ln, len(self.log_lines),
           lambda n: self.log_lines[n].split()[0] != prefix)
       words = self.log_lines[ln].split()
@@ -393,26 +403,31 @@ class Log(object):
         and an integer indicating a line number next to the version string).
 
     Raises:
-        RuntimeException for invalid dump versions.
+        ParsingException for invalid dump versions.
     """
     version = ''
 
     # Skip until an identifiable line.
     headers = ('STACKTRACES:\n', 'MMAP_STACKTRACES:\n', 'heap profile: ')
-    ln = self.skip_lines_while(
+    if len(self.log_lines) <= 0:

[M-A Ruel, 2012/04/16 12:20:55]

len() < 0?
Just use:
if not self.log_lines:

[Dai Mikurube (NOT FULLTIME), 2012/04/17 03:26:32]

Done.

+      raise ParsingException('Empty heap dump file.')
+    (ln, found) = self.skip_lines_while(
         0, len(self.log_lines),
         lambda n: not self.log_lines[n].startswith(headers))
+    if not found:
+      raise ParsingException('Invalid heap dump file (no version header).')
 
     # Identify a version.
     if self.log_lines[ln].startswith('heap profile: '):
       version = self.log_lines[ln][13:].strip()
       if (version == DUMP_DEEP_2 or version == DUMP_DEEP_3 or
           version == DUMP_DEEP_4):
-        ln = self.skip_lines_while(
+        (ln, found) = self.skip_lines_while(
             ln, len(self.log_lines),
             lambda n: self.log_lines[n] != 'MMAP_STACKTRACES:\n')
       else:
-        raise RuntimeError('invalid heap profile dump version: %s' % version)
+        raise ParsingException('invalid heap profile dump version: %s'
+                               % version)
     elif self.log_lines[ln] == 'STACKTRACES:\n':
       version = DUMP_DEEP_1
     elif self.log_lines[ln] == 'MMAP_STACKTRACES:\n':
@@ -746,8 +761,6 @@ Examples:
         buckets[int(words[0])] = Bucket(words[1:])
     n += 1
 
-  sys.stderr.write('the number buckets: %d\n' % (bucket_count))
-
   log_path_list = [log_path]
 
   if action in ('--csv', '--json'):
@@ -762,7 +775,16 @@ Examples:
         break
       n += 1
 
-  logs = [Log(path, buckets) for path in log_path_list]
+  logs = []
+  for path in log_path_list:
+    new_log = Log(path)
+    sys.stderr.write('Parsing a dump: %s\n' % path)
+    try:
+      new_log.parse_log(buckets)

[M-A Ruel, 2012/04/16 12:20:55]

The functional part of me dislike the fact that a member function 'loads' the
object and prefers something like:
new_log = Log.parse_log(path, buckets)
but I don't mind.

[Dai Mikurube (NOT FULLTIME), 2012/04/17 03:26:32]

I basically agree with you.  I'll do that in another change.

+    except ParsingException:
+      sys.stderr.write('  Ignored an invalid dump: %s\n' % path)
+    else:
+      logs.append(new_log)
 
   sys.stderr.write('getting symbols\n')
   update_symbols(symbol_path, maps_lines, chrome_path)