[opt-viewer] Add progress indicators (PR33522)

[modocache]

Summary

Provide feedback to users of opt-diff.py, opt-stats.py, and opt-viewer.py,
on how many YAML files have finished being processed, and how many HTML
files have been generated. This feedback is particularly helpful for
opt-viewer.py, which may take a long time to complete when given many
large YAML files as input.

The progress indicators use simple output such as the following:

Reading YAML files...
    9 of 1197

Test plan:
Run utils/opt-viewer/opt-*.py on a CentOS and macOS machine, using
Python 3.4 and Python 2.7 respectively, and ensure the output is
formatted well on both.

[anemet]

This is great! I am wondering if you looked at actual progressbars. For example looks like that tqdm might not be hard to adapt to multiprocessing (https://stackoverflow.com/a/40133278). What do you think? I am happy to take this as is if you don't want to spend more time on this for now. This is already great "progress" :)

[modocache]

I had seen tqdm, but:

1. I wasn't sure what you thought about taking on another dependency, so I erred on the safe side of no additional external dependencies
2. Just printing "N of 1197" is good enough for me, it lets me get some feedback on whether the script is still working hard or whether it's stuck

Up to you, though! Happy to rewrite this with tqdm. My personal preference would be to merge as-is, and if someone wants to make a nicer-looking progress bar they're free to use tqdm.

[simon.f.whittaker]

In D34735#795416, @modocache wrote:

I had seen tqdm, but:

1. I wasn't sure what you thought about taking on another dependency, so I erred on the safe side of no additional external dependencies
2. Just printing "N of 1197" is good enough for me, it lets me get some feedback on whether the script is still working hard or whether it's stuck

Up to you, though! Happy to rewrite this with tqdm. My personal preference would be to merge as-is, and if someone wants to make a nicer-looking progress bar they're free to use tqdm.

I'd probably err on the side of fewer dependencies also. Within PlayStation we're considering distributing these scripts to end-users (as a stop-gap until something we could possibly do something better with the YAML) so each additional dependency adds some pain.

FWIW I tested the change on Windows also (against Python 2.7.13 only) and it works fine.

Thanks for the work.

Simon

[anemet]

In D34735#795416, @modocache wrote:

I had seen tqdm, but:

1. I wasn't sure what you thought about taking on another dependency, so I erred on the safe side of no additional external dependencies

I was hoping we could do it the way the libYAML dependency is handled: use it as an optional library.

1. Just printing "N of 1197" is good enough for me, it lets me get some feedback on whether the script is still working hard or whether it's stuck

Up to you, though! Happy to rewrite this with tqdm. My personal preference would be to merge as-is, and if someone wants to make a nicer-looking progress bar they're free to use tqdm.

I am cool with that. LGTM then ;).

[modocache]

_Very_ minor tweak for macOS: add an extra carriage return after all processing is done. I'll land this now, thanks for the reviews, and for checking on Windows!

utils/opt-viewer/opt-diff.py

if __name__ == '__main__':
     parser.add_argument('yaml_dir_or_file_1')
     parser.add_argument('yaml_dir_or_file_2')
     parser.add_argument(
         '--jobs',
         '-j',
         default=cpu_count(),
         type=int,
         help='Max job count (defaults to %(default)s, the current CPU count)')
+    parser.add_argument(
+        '--no-progress-indicator',
+        '-n',
+        action='store_true',
+        default=False,
+        help='Do not display any indicator of how many YAML files were read.')
     parser.add_argument('--output', '-o', default='diff.opt.yaml')
     args = parser.parse_args()
 
-    if args.jobs == 1:
-        pmap = map
-    else:
-        pool = Pool(processes=args.jobs)
-        pmap = pool.map
-
     files1 = find_files(args.yaml_dir_or_file_1)
     files2 = find_files(args.yaml_dir_or_file_2)
 
-    all_remarks1, _, _ = optrecord.gather_results(pmap, files1)
-    all_remarks2, _, _ = optrecord.gather_results(pmap, files2)
+    print_progress = not args.no_progress_indicator
+    all_remarks1, _, _ = optrecord.gather_results(files1, args.jobs, print_progress)
+    all_remarks2, _, _ = optrecord.gather_results(files2, args.jobs, print_progress)
 
     added = set(all_remarks2.values()) - set(all_remarks1.values())
     removed = set(all_remarks1.values()) - set(all_remarks2.values())
 
     for r in added:
         r.Added = True
     for r in removed:
         r.Added = False
     with open(args.output, 'w') as stream:
         yaml.dump_all(added | removed, stream)

utils/opt-viewer/opt-stats.py

if __name__ == '__main__':
     parser = argparse.ArgumentParser(description=desc)
     parser.add_argument('yaml_files', nargs='+')
     parser.add_argument(
         '--jobs',
         '-j',
         default=cpu_count(),
         type=int,
         help='Max job count (defaults to %(default)s, the current CPU count)')
+    parser.add_argument(
+        '--no-progress-indicator',
+        '-n',
+        action='store_true',
+        default=False,
+        help='Do not display any indicator of how many YAML files were read.')
     args = parser.parse_args()
 
-    if args.jobs == 1:
-        pmap = map
-    else:
-        pool = Pool(processes=args.jobs)
-        pmap = pool.map
-
-    all_remarks, file_remarks, _ = optrecord.gather_results(pmap, args.yaml_files)
+    print_progress = not args.no_progress_indicator
+    all_remarks, file_remarks, _ = optrecord.gather_results(
+        args.yaml_files, args.jobs, print_progress)
+    if print_progress:
+        print('\n')
 
     bypass = defaultdict(int)
     byname = defaultdict(int)
     for r in optrecord.itervalues(all_remarks):
         bypass[r.Pass] += 1
         byname[r.Pass + "/" + r.Name] += 1
 
     total = len(all_remarks)

utils/opt-viewer/opt-viewer.py

 #!/usr/bin/env python2.7
 
 from __future__ import print_function
 
-desc = '''Generate HTML output to visualize optimization records from the YAML files
-generated with -fsave-optimization-record and -fdiagnostics-show-hotness.
-
-The tools requires PyYAML and Pygments Python packages.'''
-
-import optrecord
-import functools
-from multiprocessing import Pool
-from multiprocessing import Lock, cpu_count
-import errno
 import argparse
+import cgi
+import errno
+import functools
+from multiprocessing import cpu_count
 import os.path
 import re
 import shutil
+
 from pygments import highlight
 from pygments.lexers.c_cpp import CppLexer
 from pygments.formatters import HtmlFormatter
-import cgi
+
+import optpmap
+import optrecord
+
+
+desc = '''Generate HTML output to visualize optimization records from the YAML files
+generated with -fsave-optimization-record and -fdiagnostics-show-hotness.
+
+The tools requires PyYAML and Pygments Python packages.'''
+
 
 # This allows passing the global context to the child processes.
 class Context:
     def __init__(self, caller_loc = dict()):
        # Map function names to their source location for function where inlining happened
        self.caller_loc = caller_loc
 
 context = Context()
def map_remarks(all_remarks):
     for remark in optrecord.itervalues(all_remarks):
         if isinstance(remark, optrecord.Passed) and remark.Pass == "inline" and remark.Name == "Inlined":
             for arg in remark.Args:
                 caller = arg.get('Caller')
                 if caller:
                     context.caller_loc[caller] = arg['DebugLoc']
 
 
-def generate_report(pmap, all_remarks, file_remarks, source_dir, output_dir, should_display_hotness):
+def generate_report(all_remarks,
+                    file_remarks,
+                    source_dir,
+                    output_dir,
+                    should_display_hotness,
+                    num_jobs,
+                    should_print_progress):
     try:
         os.makedirs(output_dir)
     except OSError as e:
         if e.errno == errno.EEXIST and os.path.isdir(output_dir):
             pass
         else:
             raise
 
     _render_file_bound = functools.partial(_render_file, source_dir, output_dir, context)
-    pmap(_render_file_bound, file_remarks.items())
+    if should_print_progress:
+        print('Rendering HTML files...')
+    optpmap.pmap(_render_file_bound,
+                 file_remarks.items(),
+                 num_jobs,
+                 should_print_progress)
 
     if should_display_hotness:
         sorted_remarks = sorted(optrecord.itervalues(all_remarks), key=lambda r: (r.Hotness, r.File, r.Line, r.Column, r.PassWithDiffPrefix, r.yaml_tag, r.DemangledFunctionName), reverse=True)
     else:
         sorted_remarks = sorted(optrecord.itervalues(all_remarks), key=lambda r: (r.File, r.Line, r.Column, r.PassWithDiffPrefix, r.yaml_tag, r.DemangledFunctionName))
     IndexRenderer(args.output_dir).render(sorted_remarks)
 
     shutil.copy(os.path.join(os.path.dirname(os.path.realpath(__file__)),
parser.add_argument(
         default=cpu_count(),
         type=int,
         help='Max job count (defaults to %(default)s, the current CPU count)')
     parser.add_argument(
         '-source-dir',
         '-s',
         default='',
         help='set source directory')
+    parser.add_argument(
+        '--no-progress-indicator',
+        '-n',
+        action='store_true',
+        default=False,
+        help='Do not display any indicator of how many YAML files were read '
+             'or rendered into HTML.')
     args = parser.parse_args()
 
-    if args.jobs == 1:
-        pmap = map
-    else:
-        pool = Pool(processes=args.jobs)
-        pmap = pool.map
-
-    all_remarks, file_remarks, should_display_hotness = optrecord.gather_results(pmap, args.yaml_files)
+    print_progress = not args.no_progress_indicator
+    all_remarks, file_remarks, should_display_hotness = \
+        optrecord.gather_results(args.yaml_files, args.jobs, print_progress)
 
     map_remarks(all_remarks)
 
-    generate_report(pmap, all_remarks, file_remarks, args.source_dir, args.output_dir, should_display_hotness)
+    generate_report(all_remarks,
+                    file_remarks,
+                    args.source_dir,
+                    args.output_dir,
+                    should_display_hotness,
+                    args.jobs,
+                    print_progress)

utils/opt-viewer/optpmap.py

+import sys
+import multiprocessing
+
+
+_current = None
+_total = None
+
+
+def _init(current, total):
+    global _current
+    global _total
+    _current = current
+    _total = total
+
+
+def _wrapped_func(func_and_args):
+    func, argument, should_print_progress = func_and_args
+
+    if should_print_progress:
+        with _current.get_lock():
+            _current.value += 1
+        sys.stdout.write('\r\t{} of {}'.format(_current.value, _total.value))
+
+    return func(argument)
+
+
+def pmap(func, iterable, processes, should_print_progress, *args, **kwargs):
+    """
+    A parallel map function that reports on its progress.
+
+    Applies `func` to every item of `iterable` and return a list of the
+    results. If `processes` is greater than one, a process pool is used to run
+    the functions in parallel. `should_print_progress` is a boolean value that
+    indicates whether a string 'N of M' should be printed to indicate how many
+    of the functions have finished being run.
+    """
+    global _current
+    global _total
+    _current = multiprocessing.Value('i', 0)
+    _total = multiprocessing.Value('i', len(iterable))
+
+    func_and_args = [(func, arg, should_print_progress,) for arg in iterable]
+    if processes <= 1:
+        result = map(_wrapped_func, func_and_args, *args, **kwargs)
+    else:
+        pool = multiprocessing.Pool(initializer=_init,
+                                    initargs=(_current, _total,),
+                                    processes=processes)
+        result = pool.map(_wrapped_func, func_and_args, *args, **kwargs)
+
+    if should_print_progress:
+        sys.stdout.write('\r')
+    return result

utils/opt-viewer/optrecord.py

 #!/usr/bin/env python2.7
 
 from __future__ import print_function
 
 import yaml
 # Try to use the C parser.
 try:
     from yaml import CLoader as Loader
 except ImportError:
     print("For faster parsing, you may want to install libYAML for PyYAML")
     from yaml import Loader
 
-import functools
-from collections import defaultdict
-import itertools
-from multiprocessing import Pool
-from multiprocessing import Lock, cpu_count
 import cgi
+from collections import defaultdict
+import functools
+from multiprocessing import Lock
 import subprocess
 
-import traceback
+import optpmap
+
 
 p = subprocess.Popen(['c++filt', '-n'], stdin=subprocess.PIPE, stdout=subprocess.PIPE)
 p_lock = Lock()
 
 
 try:
     dict.iteritems
 except AttributeError:
with open(input_file) as f:
             # in the file.
             if hasattr(remark, 'max_hotness'):
                 max_hotness = remark.max_hotness
             max_hotness = max(max_hotness, remark.Hotness)
 
     return max_hotness, all_remarks, file_remarks
 
 
-def gather_results(pmap, filenames):
-    remarks = pmap(get_remarks, filenames)
+def gather_results(filenames, num_jobs, should_print_progress):
+    if should_print_progress:
+        print('Reading YAML files...')
+    remarks = optpmap.pmap(
+        get_remarks, filenames, num_jobs, should_print_progress)
     max_hotness = max(entry[0] for entry in remarks)
 
     def merge_file_remarks(file_remarks_job, all_remarks, merged):
         for filename, d in iteritems(file_remarks_job):
             for line, remarks in iteritems(d):
                 for remark in remarks:
                     # Bring max_hotness into the remarks so that
                     # RelativeHotness does not depend on an external global.