[tools] Cleanup Arguments processing in tools

- Move readFile helper to LogReader.readFile
- Add static BaseArgumentsProcessor.process helper
- Move SourceMap handling to the TickProcessor
- Always skip example file mjsunit/tools/tickprocessor-test-large.js
- Run tickprocessor and dumpcpp tests only in release mode

Change-Id: I635fb2d2839233219b058faf9710fd0f19880fd2
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2929117Reviewed-by: Marja Hölttä <marja@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74873}

test/mjsunit/BUILD.gn

@@ -17,6 +17,7 @@ group("v8_mjsunit") {
     "../../tools/clusterfuzz/v8_mock_archs.js",
     "../../tools/clusterfuzz/v8_mock_webassembly.js",
     "../../tools/codemap.mjs",
+    "../../tools/sourcemap.mjs",
     "../../tools/consarray.mjs",
     "../../tools/csvparser.mjs",
     "../../tools/dumpcpp.mjs",

test/mjsunit/mjsunit.status

@@ -83,8 +83,10 @@
   'regress/regress-crbug-491062': [PASS, NO_VARIANTS],
   'third_party/regexp-pcre/regexp-pcre': [PASS, NO_VARIANTS],
   'tools/compiler-trace-flags': [PASS, NO_VARIANTS],
-  'tools/dumpcpp': [PASS, NO_VARIANTS],
-  'tools/tickprocessor': [PASS, NO_VARIANTS],
+  'tools/dumpcpp': [PASS, NO_VARIANTS, ['mode != release', SKIP]],
+  'tools/tickprocessor': [PASS, NO_VARIANTS, ['mode != release', SKIP]],
+   # Also skip example file which is not a test.
+  'tools/tickprocessor-test-large': [SKIP],
 
   # Issue 488: this test sometimes times out.
   # TODO(arm): This seems to flush out a bug on arm with simulator.
@@ -147,7 +149,6 @@
   'wasm/atomics64-stress': [PASS, SLOW, NO_VARIANTS, ['mode != release or dcheck_always_on', SKIP], ['tsan', SKIP]],
   'wasm/compare-exchange-stress': [PASS, SLOW, NO_VARIANTS],
   'wasm/compare-exchange64-stress': [PASS, SLOW, NO_VARIANTS],
-  'tools/tickprocessor-test-large': [PASS, SLOW, NO_VARIANTS, ['mode != release or arch != x64', SKIP]],
 
   # Very slow on ARM and MIPS, contains no architecture dependent code.
   'unicode-case-overoptimization0': [PASS, NO_VARIANTS, ['arch in (arm, arm64, mipsel, mips64el, mips64, mips, riscv64)', SKIP]],

test/mjsunit/tools/tickprocessor.mjs

@@ -44,9 +44,18 @@
 
 import {
   TickProcessor, ArgumentsProcessor, UnixCppEntriesProvider,
-  MacCppEntriesProvider, WindowsCppEntriesProvider, readFile
+  MacCppEntriesProvider, WindowsCppEntriesProvider 
 } from "../../../tools/tickprocessor.mjs";
 
+// TODO(cbruni): Remove after refactoring
+export function readFile(fileName) {
+  try {
+    return read(fileName);
+  } catch (e) {
+    console.log(fileName + ': ' + (e.message || e));
+    throw e;
+  }
+}
 
 (function testArgumentsProcessor() {
   var p_default = new ArgumentsProcessor([]);

tools/arguments.mjs

@@ -22,6 +22,16 @@ export class BaseArgumentsProcessor {
 
   result() { return this.result_ }
 
+  static process(args) {
+    const processor = new this(args);
+    if (processor.parse()) {
+      return processor.result();
+    } else {
+      processor.printUsageAndExit();
+      return false;
+    }
+  }
+
   printUsageAndExit() {
     console.log('Cmdline args: [options] [log-file-name]\n' +
           'Default log file name is "' +

tools/dumpcpp-driver.mjs

@@ -2,7 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import { WebInspector } from "./sourcemap.mjs";
 import {
     CppProcessor, ArgumentsProcessor, UnixCppEntriesProvider,
     WindowsCppEntriesProvider, MacCppEntriesProvider
@@ -10,40 +9,13 @@ import {
 
 // Dump C++ symbols of shared library if possible
 
-function processArguments(args) {
-  const processor = new ArgumentsProcessor(args);
-  if (processor.parse()) {
-    return processor.result();
-  } else {
-    processor.printUsageAndExit();
-  }
-}
-
-function initSourceMapSupport() {
-  // Pull dev tools source maps into our name space.
-  SourceMap = WebInspector.SourceMap;
-
-  // Overwrite the load function to load scripts synchronously.
-  SourceMap.load = function(sourceMapURL) {
-    const content = readFile(sourceMapURL);
-    const sourceMapObject = (JSON.parse(content));
-    return new SourceMap(sourceMapURL, sourceMapObject);
-  };
-}
-
 const entriesProviders = {
   'unix': UnixCppEntriesProvider,
   'windows': WindowsCppEntriesProvider,
   'mac': MacCppEntriesProvider
 };
 
-const params = processArguments(arguments);
-let sourceMap = null;
-if (params.sourceMap) {
-  initSourceMapSupport();
-  sourceMap = SourceMap.load(params.sourceMap);
-}
-
+const params = ArgumentsProcessor.process(arguments);
 const cppProcessor = new CppProcessor(
   new (entriesProviders[params.platform])(params.nm, params.targetRootFS,
                                           params.apkEmbeddedLibrary),

tools/dumpcpp.mjs

@@ -41,7 +41,7 @@ export class CppProcessor extends LogReader {
   processLogFileInTest(fileName) {
     // Hack file name to avoid dealing with platform specifics.
     this.lastLogFileName_ = 'v8.log';
-    const contents = readFile(fileName);
+    const contents = this.readFile(fileName);
     this.processLogChunk(contents);
   }
 

tools/ic-processor-driver.mjs

@@ -3,49 +3,13 @@
 // found in the LICENSE file.
 
 import { Processor } from "./system-analyzer/processor.mjs";
-import { WebInspector } from "./sourcemap.mjs";
-import { BaseArgumentsProcessor } from "./arguments.mjs";
-
-function processArguments(args) {
-  const processor = new ArgumentsProcessor(args);
-  if (processor.parse()) {
-    return processor.result();
-  } else {
-    processor.printUsageAndExit();
-  }
-}
-
-/**
- * A thin wrapper around shell's 'read' function showing a file name on error.
- */
-export function readFile(fileName) {
-  try {
-    return read(fileName);
-  } catch (e) {
-    console.log(fileName + ': ' + (e.message || e));
-    throw e;
-  }
-}
-
-function initSourceMapSupport() {
-  // Pull dev tools source maps into our name space.
-  SourceMap = WebInspector.SourceMap;
-
-  // Overwrite the load function to load scripts synchronously.
-  SourceMap.load = function(sourceMapURL) {
-    const content = readFile(sourceMapURL);
-    const sourceMapObject = (JSON.parse(content));
-    return new SourceMap(sourceMapURL, sourceMapObject);
-  };
-}
+import { BaseArgumentsProcessor} from "./arguments.mjs";
 
 class ArgumentsProcessor extends BaseArgumentsProcessor {
   getArgsDispatch() {
     return {
       '--range': ['range', 'auto,auto',
           'Specify the range limit as [start],[end]'],
-      '--source-map': ['sourceMap', null,
-          'Specify the source map that should be used for output']
     };
   }
   getDefaultResults() {
@@ -56,26 +20,21 @@ class ArgumentsProcessor extends BaseArgumentsProcessor {
   }
 }
 
-const params = processArguments(arguments);
-let sourceMap = null;
-if (params.sourceMap) {
-  initSourceMapSupport();
-  sourceMap = SourceMap.load(params.sourceMap);
-}
+const params = ArgumentsProcessor.process(arguments);
 const processor = new Processor();
 processor.processLogFile(params.logFileName);
 
 const typeAccumulator = new Map();
 
 const accumulator = {
-  __proto__: null, 
+  __proto__: null,
   LoadGlobalIC: 0,
   StoreGlobalIC: 0,
   LoadIC: 0,
   StoreIC: 0,
   KeyedLoadIC: 0,
   KeyedStoreIC: 0,
-  StoreInArrayLiteralIC: 0, 
+  StoreInArrayLiteralIC: 0,
 }
 for (const ic of processor.icTimeline.all) {
   console.log(Object.values(ic));
@@ -86,5 +45,3 @@ console.log("========================================");
 for (const key of Object.keys(accumulator)) {
   console.log(key + ": " + accumulator[key]);
 }
-
-

tools/logreader.mjs

@@ -90,6 +90,19 @@ export class LogReader {
     this.logLinesSinceLastTimerMarker_ = [];
   }
 
+
+  /**
+   * A thin wrapper around shell's 'read' function showing a file name on error.
+   */
+  readFile(fileName) {
+    try {
+      return read(fileName);
+    } catch (e) {
+      printErr(`file="${fileName}": ${e.message || e}`);
+      throw e;
+    }
+  }
+
   /**
    * Used for printing error messages.
    *

tools/parse-processor-driver.mjs

@@ -2,37 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import { WebInspector } from "./sourcemap.mjs";
-import {
-    ParseProcessor, ArgumentsProcessor, readFile,
-  } from "./parse-processor.mjs";
+import { ParseProcessor, ArgumentsProcessor } from "./parse-processor.mjs";
 
-function processArguments(args) {
-  const processor = new ArgumentsProcessor(args);
-  if (processor.parse()) {
-    return processor.result();
-  } else {
-    processor.printUsageAndExit();
-  }
-}
-
-function initSourceMapSupport() {
-  // Pull dev tools source maps  into our name space.
-  SourceMap = WebInspector.SourceMap;
-
-  // Overwrite the load function to load scripts synchronously.
-  SourceMap.load = function(sourceMapURL) {
-    const content = readFile(sourceMapURL);
-    const sourceMapObject = (JSON.parse(content));
-    return new SourceMap(sourceMapURL, sourceMapObject);
-  };
-}
-
-const params = processArguments(arguments);
-let sourceMap = null;
-if (params.sourceMap) {
-  initSourceMapSupport();
-  sourceMap = SourceMap.load(params.sourceMap);
-}
+const params = ArgumentsProcessor.process(arguments);
 const parseProcessor = new ParseProcessor();
 parseProcessor.processLogFile(params.logFileName);

tools/parse-processor.mjs

@@ -4,18 +4,6 @@
 import { LogReader, parseString } from "./logreader.mjs";
 import { BaseArgumentsProcessor } from "./arguments.mjs";
 
-/**
- * A thin wrapper around shell's 'read' function showing a file name on error.
- */
-export function readFile(fileName) {
-  try {
-    return read(fileName);
-  } catch (e) {
-    console.log(fileName + ': ' + (e.message || e));
-    throw e;
-  }
-}
-
 // ===========================================================================
 
 // This is the only true formatting, why? For an international audience the

tools/tickprocessor-driver.mjs

@@ -25,47 +25,20 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-import { WebInspector} from "./sourcemap.mjs";
 import {
     ArgumentsProcessor, TickProcessor, UnixCppEntriesProvider,
-    WindowsCppEntriesProvider, MacCppEntriesProvider, readFile,
+    WindowsCppEntriesProvider, MacCppEntriesProvider
   } from "./tickprocessor.mjs";
 
 // Tick Processor's code flow.
 
-function processArguments(args) {
-  const processor = new ArgumentsProcessor(args);
-  if (processor.parse()) {
-    return processor.result();
-  } else {
-    processor.printUsageAndExit();
-  }
-}
-
-function initSourceMapSupport() {
-  // Pull dev tools source maps  into our name space.
-  SourceMap = WebInspector.SourceMap;
-
-  // Overwrite the load function to load scripts synchronously.
-  SourceMap.load = function(sourceMapURL) {
-    const content = readFile(sourceMapURL);
-    const sourceMapObject = (JSON.parse(content));
-    return new SourceMap(sourceMapURL, sourceMapObject);
-  };
-}
-
 const entriesProviders = {
   'unix': UnixCppEntriesProvider,
   'windows': WindowsCppEntriesProvider,
   'mac': MacCppEntriesProvider
 };
 
-const params = processArguments(arguments);
-let sourceMap = null;
-if (params.sourceMap) {
-  initSourceMapSupport();
-  sourceMap = SourceMap.load(params.sourceMap);
-}
+const params = ArgumentsProcessor.process(arguments);
 const tickProcessor = new TickProcessor(
   new (entriesProviders[params.platform])(params.nm, params.objdump, params.targetRootFS,
                                           params.apkEmbeddedLibrary),
@@ -79,7 +52,7 @@ const tickProcessor = new TickProcessor(
   params.stateFilter,
   params.distortion,
   params.range,
-  sourceMap,
+  params.sourceMap,
   params.timedRange,
   params.pairwiseTimedRange,
   params.onlySummary,

tools/tickprocessor.mjs

@@ -29,6 +29,7 @@ import { LogReader, parseString, parseVarArgs } from "./logreader.mjs";
 import { BaseArgumentsProcessor, parseBool } from "./arguments.mjs";
 import { Profile, JsonProfile } from "./profile.mjs";
 import { ViewBuilder } from "./profile_view.mjs";
+import { WebInspector} from "./sourcemap.mjs";
 
 
 class V8Profile extends Profile {
@@ -59,19 +60,6 @@ class V8Profile extends Profile {
 }
 
 
-/**
- * A thin wrapper around shell's 'read' function showing a file name on error.
- */
-export function readFile(fileName) {
-  try {
-    return read(fileName);
-  } catch (e) {
-    printErr(`file="${fileName}": ${e.message || e}`);
-    throw e;
-  }
-}
-
-
 export class TickProcessor extends LogReader {
   constructor(
     cppEntriesProvider,
@@ -174,7 +162,7 @@ export class TickProcessor extends LogReader {
     this.ignoreUnknown_ = ignoreUnknown;
     this.stateFilter_ = stateFilter;
     this.runtimeTimerFilter_ = runtimeTimerFilter;
-    this.sourceMap = sourceMap;
+    this.sourceMap = this.loadSourceMap(sourceMap);
     const ticks = this.ticks_ =
       { total: 0, unaccounted: 0, excluded: 0, gc: 0 };
 
@@ -226,6 +214,17 @@ export class TickProcessor extends LogReader {
     this.onlySummary_ = onlySummary;
   }
 
+  loadSourceMap(sourceMap) {
+    if (!sourceMap) return null;
+    // Overwrite the load function to load scripts synchronously.
+    WebInspector.SourceMap.load = (sourceMapURL) => {
+      const content = this.readFile(sourceMapURL);
+      const sourceMapObject = JSON.parse(content);
+      return new SourceMap(sourceMapURL, sourceMapObject);
+    };
+    return WebInspector.SourceMap.load(sourceMap);
+  }
+
   static VmStates = {
     JS: 0,
     GC: 1,
@@ -282,7 +281,7 @@ export class TickProcessor extends LogReader {
   processLogFileInTest(fileName) {
     // Hack file name to avoid dealing with platform specifics.
     this.lastLogFileName_ = 'v8.log';
-    const contents = readFile(fileName);
+    const contents = this.readFile(fileName);
     this.processLogChunk(contents);
   }
 
@@ -835,6 +834,7 @@ export class WindowsCppEntriesProvider extends CppEntriesProvider {
 export class ArgumentsProcessor extends BaseArgumentsProcessor {
   getArgsDispatch() {
     let dispatch = {
+      __proto__:null,
       '-j': ['stateFilter', TickProcessor.VmStates.JS,
         'Show only ticks from JS VM state'],
       '-g': ['stateFilter', TickProcessor.VmStates.GC,
@@ -918,6 +918,7 @@ export class ArgumentsProcessor extends BaseArgumentsProcessor {
       separateStubs: true,
       separateBaselineHandlers: false,
       preprocessJson: null,
+      sourceMap: null,
       targetRootFS: '',
       nm: 'nm',
       objdump: 'objdump',