Reland "Cleanup tools/emprofile.py and add more testing. NFC (#13403)" (#13416)

Run the new test on windows to prove that it works there.

This was reverted in 13410

.circleci/config.yml

@@ -428,7 +428,7 @@ jobs:
       # note we do *not* build all libraries and freeze the cache; as we run
       # only limited tests here, it's more efficient to build on demand
       - run-tests:
-          test_targets: "other.test_emcc_cflags other.test_stdin other.test_bad_triple wasm2.test_sse1 wasm2.test_ccall other.test_closure_externs other.test_binaryen_debug other.test_js_optimizer_parse_error other.test_output_to_nowhere other.test_emcc_dev_null other.test_cmake* other.test_system_include_paths other.test_emar_response_file wasm2.test_utf16 other.test_special_chars_in_arguments"
+          test_targets: "other.test_emcc_cflags other.test_stdin other.test_bad_triple wasm2.test_sse1 wasm2.test_ccall other.test_closure_externs other.test_binaryen_debug other.test_js_optimizer_parse_error other.test_output_to_nowhere other.test_emcc_dev_null other.test_cmake* other.test_system_include_paths other.test_emar_response_file wasm2.test_utf16 other.test_special_chars_in_arguments other.test_toolchain_profiler"
   test-mac:
     executor: mac
     environment:

tests/test_other.py

@@ -47,6 +47,7 @@ scons_path = utils.which('scons')
 emmake = shared.bat_suffix(path_from_root('emmake'))
 emconfig = shared.bat_suffix(path_from_root('em-config'))
 emsize = shared.bat_suffix(path_from_root('emsize'))
+emprofile = shared.bat_suffix(path_from_root('emprofile'))
 wasm_dis = Path(building.get_binaryen_bin(), 'wasm-dis')
 wasm_opt = Path(building.get_binaryen_bin(), 'wasm-opt')
 
@@ -7795,9 +7796,19 @@ end
     self.run_process([EMCC, test, '--closure=1', '--closure-args', '--externs "' + externs + '"'])
 
   def test_toolchain_profiler(self):
-    with env_modify({'EMPROFILE': '1'}):
-      # replaced subprocess functions should not cause errors
-      self.run_process([EMCC, test_file('hello_world.c')])
+    # Verify some basic functionality of EMPROFILE
+    environ = os.environ.copy()
+    environ['EMPROFILE'] = '1'
+
+    self.run_process([emprofile, '--reset'])
+    err = self.expect_fail([emprofile, '--graph'])
+    self.assertContained('No profiler logs were found', err)
+
+    self.run_process([EMCC, test_file('hello_world.c')], env=environ)
+    self.assertEqual('hello, world!', self.run_js('a.out.js').strip())
+
+    self.run_process([emprofile, '--graph'])
+    self.assertTrue(glob.glob('toolchain_profiler.results*.html'))
 
   def test_noderawfs(self):
     fopen_write = read_file(test_file('asmfs/fopen_write.cpp'))

tools/emprofile.bat

@@ -1,2 +0,0 @@
-@echo off
-python "%~dp0\emprofile.py" %*

tools/emprofile.py

@@ -15,75 +15,64 @@ from pathlib import Path
 
 profiler_logs_path = os.path.join(tempfile.gettempdir(), 'emscripten_toolchain_profiler_logs')
 
-OUTFILE = 'emprofile.' + time.strftime('%Y%m%d_%H%M')
-for i in range(len(sys.argv)):
-  arg = sys.argv[i]
-  if arg.startswith('--outfile=') or arg.startswith('-o='):
-    OUTFILE = arg.split('=', 1)[1].strip().replace('.html', '')
-    sys.argv[i] = ''
-  elif arg == '-o':
-    OUTFILE = sys.argv[i + 1].strip().replace('.html', '')
-    sys.argv[i] = sys.argv[i + 1] = ''
-
 
 # Deletes all previously captured log files to make room for a new clean run.
 def delete_profiler_logs():
-  try:
+  if os.path.exists(profiler_logs_path):
     shutil.rmtree(profiler_logs_path)
-  except IOError:
-    pass
 
 
 def list_files_in_directory(d):
   files = []
-  try:
-    items = os.listdir(d)
-    for i in items:
+  if os.path.exists(d):
+    for i in os.listdir(d):
       f = os.path.join(d, i)
       if os.path.isfile(f):
-        files += [f]
-    return files
-  except IOError:
-    return []
+        files.append(f)
+  return files
 
 
-def create_profiling_graph():
+def create_profiling_graph(outfile):
   log_files = [f for f in list_files_in_directory(profiler_logs_path) if 'toolchain_profiler.pid_' in f]
 
   all_results = []
   if len(log_files):
-    print('Processing ' + str(len(log_files)) + ' profile log files in "' + profiler_logs_path + '"...')
+    print(f'Processing {len(log_files)} profile log files in {profiler_logs_path}...')
   for f in log_files:
+    print(f'Processing: {f}')
+    json_data = Path(f).read_text()
+    if len(json_data.strip()) == 0:
+      continue
+    lines = json_data.split('\n')
+    lines = [x for x in lines if x != '[' and x != ']' and x != ',' and len(x.strip())]
+    lines = [(x + ',') if not x.endswith(',') else x for x in lines]
+    lines[-1] = lines[-1][:-1]
+    json_data = '[' + '\n'.join(lines) + ']'
     try:
-      json_data = Path(f).read_text()
-      if len(json_data.strip()) == 0:
-        continue
-      lines = json_data.split('\n')
-      lines = [x for x in lines if x != '[' and x != ']' and x != ',' and len(x.strip())]
-      lines = [(x + ',') if not x.endswith(',') else x for x in lines]
-      lines[-1] = lines[-1][:-1]
-      json_data = '[' + '\n'.join(lines) + ']'
       all_results += json.loads(json_data)
-    except Exception as e:
+    except json.JSONDecodeError as e:
       print(str(e), file=sys.stderr)
       print('Failed to parse JSON file "' + f + '"!', file=sys.stderr)
-      sys.exit(1)
+      return 1
   if len(all_results) == 0:
-    print('No profiler logs were found in path "' + profiler_logs_path + '". Try setting the environment variable EMPROFILE=1 and run some emcc commands, and then rerun "emprofile" again.')
-    return
+    print(f'No profiler logs were found in path: ${profiler_logs_path}.\nTry setting the environment variable EMPROFILE=1 and run some emcc commands, then re-run "emprofile.py --graph".', file=sys.stderr)
+    return 1
 
   all_results.sort(key=lambda x: x['time'])
 
   emprofile_json_data = json.dumps(all_results, indent=2)
 
-  html_file = OUTFILE + '.html'
+  html_file = outfile + '.html'
   html_contents = Path(os.path.dirname(os.path.realpath(__file__)), 'toolchain_profiler.results_template.html').read_text().replace('{{{ emprofile_json_data }}}', emprofile_json_data)
   Path(html_file).write_text(html_contents)
-  print('Wrote "' + html_file + '"')
+  print(f'Wrote "{html_file}"')
+  return 0
 
 
-if '--help' in sys.argv:
-  print('''Usage:
+def main(args):
+  if '--help' in args:
+    print('''\
+Usage:
        emprofile.py --clear (or -c)
          Deletes all previously recorded profiling log files.
          Use this to abort/drop any previously collected
@@ -99,12 +88,25 @@ Optional parameters:
         --outfile=x.html (or -o=x.html)
           Specifies the name of the results file to generate.
 ''')
-  sys.exit(1)
+    return 0
 
-
-if '--reset' in sys.argv or '--clear' in sys.argv or '-c' in sys.argv:
-  delete_profiler_logs()
-else:
-  create_profiling_graph()
-  if '--no-clear' not in sys.argv:
+  if '--reset' in args or '--clear' in args or '-c' in args:
     delete_profiler_logs()
+    return 0
+  else:
+    outfile = 'toolchain_profiler.results_' + time.strftime('%Y%m%d_%H%M')
+    for i, arg in enumerate(args):
+      if arg.startswith('--outfile=') or arg.startswith('-o='):
+        outfile = arg.split('=', 1)[1].strip().replace('.html', '')
+      elif arg == '-o':
+        outfile = args[i + 1].strip().replace('.html', '')
+    if create_profiling_graph(outfile):
+      return 1
+    if '--no-clear' not in args:
+      delete_profiler_logs()
+
+  return 0
+
+
+if __name__ == '__main__':
+  sys.exit(main(sys.argv[1:]))