Merge pull request #395 from jpmorganchase/optimize_main

Benchmark limit + minor changes to DataAccessor
finos · Jan 23, 2019 · ec82fb0 · ec82fb0
2 parents 2a59ea7 + e719e82
commit ec82fb0
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 11 deletions.
diff --git a/docs/development.md b/docs/development.md
@@ -152,9 +152,14 @@ with your changes to preserve them for future comparison.
 yarn bench
 ```
 
+Use the `--limit <NUMBER>` flag to control the number of Perspective versions that the
+benchmark suite will run, where `<NUMBER>` is an integer greater than 0. If `<NUMBER>` 
+cannot be parsed, is 0, or is greater than the number of versions, the benchmark suite
+will run all previous versions of Perspective.
+
 The benchmarks report and `results.json` show a historgram of current
 performance, as well as that of the previous `results.json`.  Running this
 should probably be standard practice after making a large change which may
 affect performance, but please create a baseline `results.json` entry for your
-test machine on a commit before your changes first, such that the affect of your
+test machine on a commit before your changes first, such that the effects of your
 PR can be properly compared.
diff --git a/packages/perspective/bench/js/bench.js b/packages/perspective/bench/js/bench.js
@@ -11,6 +11,9 @@ const puppeteer = require("puppeteer");
 const fs = require("fs");
 const path = require("path");
 
+const args = process.argv.slice(2);
+const LIMIT = args.indexOf("--limit");
+
 const multi_template = (xs, ...ys) => ys[0].map((y, i) => [y, xs.reduce((z, x, ix) => (ys[ix] ? z + x + ys[ix][i] : z + x), "")]);
 
 const UNPKG_VERSIONS = ["0.2.11", "0.2.10", "0.2.9", "0.2.8", "0.2.7", "0.2.6", "0.2.5", "0.2.4", "0.2.3", "0.2.2", "0.2.1", "0.2.0"];
@@ -46,9 +49,20 @@ function transpose(json) {
 }
 
 async function run() {
+    // Allow users to set a limit on version lookbacks
+    let psp_urls = URLS;
+    console.log(`limit: ${LIMIT}`);
+    if (LIMIT !== -1) {
+        let limit_num = Number(args[LIMIT + 1]);
+        if (!isNaN(limit_num) && limit_num > 0 && limit_num <= psp_urls.length) {
+            console.log(`Benchmarking the last ${limit_num} versions`);
+            psp_urls = URLS.slice(0, limit_num);
+        }
+    }
+
     let data = [],
         version_index = 1;
-    for (let [version, url] of URLS) {
+    for (let [version, url] of psp_urls) {
         let browser = await puppeteer.launch({
             headless: true,
             args: ["--auto-open-devtools-for-tabs", "--no-sandbox"]

diff --git a/scripts/bench.js b/scripts/bench.js
@@ -12,6 +12,7 @@ const execSync = require("child_process").execSync;
 const execute = cmd => execSync(cmd, {stdio: "inherit"});
 
 const args = process.argv.slice(2);
+const LIMIT = args.indexOf("--limit");
 
 function docker() {
     console.log("Creating puppeteer docker image");
@@ -20,6 +21,10 @@ function docker() {
         cmd += ` --cpus="${parseInt(process.env.PSP_CPU_COUNT)}.0"`;
     }
     cmd += " perspective/puppeteer nice -n -20 node packages/perspective/bench/js/bench.js";
+    if (LIMIT !== -1) {
+        let limit = args[LIMIT + 1];
+        cmd += ` --limit ${limit}`;
+    }
     return cmd;
 }
 

diff --git a/src/cpp/emscripten.cpp b/src/cpp/emscripten.cpp
@@ -1030,7 +1030,7 @@ infer_type(val x, val date_validator) {
 }
 
 t_dtype
-get_data_type(val data, std::int32_t format, std::string name, val date_validator) {
+get_data_type(val data, std::int32_t format, const std::string& name, val date_validator) {
     std::int32_t i = 0;
     boost::optional<t_dtype> inferredType;
 
@@ -1069,7 +1069,7 @@ get_data_type(val data, std::int32_t format, std::string name, val date_validato
 }
 
 std::vector<t_dtype>
-data_types(val data, std::int32_t format, std::vector<std::string> names, val date_validator) {
+data_types(val data, std::int32_t format, const std::vector<std::string>& names, val date_validator) {
     if (names.size() == 0) {
         PSP_COMPLAIN_AND_ABORT("Cannot determine data types without column names!");
     }
@@ -1081,9 +1081,8 @@ data_types(val data, std::int32_t format, std::vector<std::string> names, val da
         std::vector<std::string> data_names
             = vecFromArray<val, std::string>(keys);
 
-        for (std::vector<std::string>::iterator name = data_names.begin();
-             name != data_names.end(); ++name) {
-            std::string value = data[*name].as<std::string>();
+        for (const std::string& name : data_names) {
+            std::string value = data[name].as<std::string>();
             t_dtype type;
 
             if (value == "integer") {
@@ -1099,17 +1098,16 @@ data_types(val data, std::int32_t format, std::vector<std::string> names, val da
             } else if (value == "date") {
                 type = t_dtype::DTYPE_DATE;
             } else {
-                PSP_COMPLAIN_AND_ABORT("Unknown type '" + value + "' for key '" + *name + "'");
+                PSP_COMPLAIN_AND_ABORT("Unknown type '" + value + "' for key '" + name + "'");
             }
 
             types.push_back(type);
         }
 
         return types;
     } else {
-        for (std::vector<std::string>::iterator name = names.begin(); name != names.end();
-             ++name) {
-            t_dtype type = get_data_type(data, format, *name, date_validator);
+        for (const std::string& name : names) {
+            t_dtype type = get_data_type(data, format, name, date_validator);
             types.push_back(type);
         }
     }