Run the next BrowserStack benchmark immediately instead of in batches (#6717)

Before this change, the benchmark script would start x benchmarks, wait for them all to complete, and then start the next x. This change makes it start new benchmarks immediately when prior benchmarks complete so that there are always x benchmarks running.

This doesn't work across tests that require different karma configs (e.g. changing model / parameters), and all tests from one karma config must complete before tests with a different karma config can start.

Co-authored-by: Linchenn <[email protected]>

Author: mattsoulanille

e2e/benchmarks/browserstack-benchmark/app.js

@@ -29,6 +29,7 @@ const {
   firebaseConfig,
   endFirebaseInstance
 } = require('./firestore.js');
+const {PromiseQueue} = require('./promise_queue');
 
 const port = process.env.PORT || 8001;
 let io;
@@ -160,22 +161,18 @@ async function benchmark(config, runOneBenchmark = getOneBenchmarkResult) {
         `dependencies...`);
   }
 
+  const promiseQueue = new PromiseQueue(cliArgs?.maxBenchmarks ?? 9);
   const results = [];
-  let numActiveBenchmarks = 0;
+
   // Runs and gets result of each queued benchmark
   for (const tabId in config.browsers) {
-    numActiveBenchmarks++;
-    results.push(runOneBenchmark(tabId, cliArgs?.maxTries).then((value) => {
-      value.deviceInfo = config.browsers[tabId];
-      value.modelInfo = config.benchmark;
-      return value;
+    results.push(promiseQueue.add(() => {
+      return runOneBenchmark(tabId, cliArgs?.maxTries).then((value) => {
+	value.deviceInfo = config.browsers[tabId];
+	value.modelInfo = config.benchmark;
+	return value;
+      });
     }));
-
-    // Waits for specified # of benchmarks to complete before running more
-    if (cliArgs?.maxBenchmarks && numActiveBenchmarks >= cliArgs.maxBenchmarks) {
-      numActiveBenchmarks = 0;
-      await Promise.allSettled(results);
-    }
   }
 
   // Optionally written to an outfile or pushed to a database once all

e2e/benchmarks/browserstack-benchmark/app_node_test.js

@@ -9,6 +9,7 @@ const {
   runFirestore,
   firebaseConfig
 } = require('./firestore.js');
+const {PromiseQueue} = require('./promise_queue.js');
 
 describe('test app.js cli', () => {
   const filePath = './benchmark_test_results.json';
@@ -257,3 +258,75 @@ describe('test adding to firestore', () => {
     }
   });
 });
+
+function sleep(n) {
+  return new Promise((resolve) => {
+    setTimeout(() => { resolve(); }, n);
+  });
+}
+
+describe('promise queue', () => {
+  let queue;
+  beforeEach(() => {
+    queue = new PromiseQueue(3);
+    jasmine.clock().install();
+  });
+
+  afterEach(() => {
+    jasmine.clock().uninstall();
+  });
+
+  it('runs a given number of functions at once', async () => {
+    let promises = [];
+    let started = [false, false, false, false, false];
+    let resolved = [false, false, false, false, false];
+    for (let i = 0; i < 5; i++) {
+      resolved[i] = false;
+      promises.push(queue.add(async () => {
+        started[i] = true;
+        await sleep((i + 1) * 10);
+        resolved[i] = true;
+      }));
+    }
+
+    // Queue should immediately start 3 promises.
+    expect(started).toEqual(
+        [true, true, true, false, false]
+    );
+    expect(resolved).toEqual(
+        [false, false, false, false, false]
+    );
+
+    // After the first promise is done, queue should start the fourth one.
+    jasmine.clock().tick(15);
+    await promises[0];
+    expect(started).toEqual(
+        [true, true, true, true, false]
+    );
+    expect(resolved).toEqual(
+        [true, false, false, false, false]
+    );
+
+    // All running promises should finish, and the last should start.
+    jasmine.clock().tick(1000);
+    await promises[1];
+    await promises[2];
+    await promises[3];
+    expect(started).toEqual(
+        [true, true, true, true, true]
+    );
+    expect(resolved).toEqual(
+        [true, true, true, true, false]
+    );
+
+    // The last promise should finish
+    jasmine.clock().tick(1000);
+    await promises[4];
+    expect(started).toEqual(
+        [true, true, true, true, true]
+    );
+    expect(resolved).toEqual(
+        [true, true, true, true, true]
+    );
+  });
+});

e2e/benchmarks/browserstack-benchmark/karma.conf.js

@@ -63,7 +63,7 @@ module.exports = function(config) {
     'tfjs-automl/dist/tf-automl.js',
   ].map(url => {
     let name = url.split('/')[0].replace('tfjs-', '').replace('backend-', '');
-    if (config.localBuild.includes(name)) {
+    if (config.localBuild?.includes(name)) {
       return `../../../dist/bin/${url}`;
     } else {
       return `https://unpkg.com/@tensorflow/${url.replace('/', '@latest/')}`;

e2e/benchmarks/browserstack-benchmark/promise_queue.js

@@ -0,0 +1,31 @@
+class PromiseQueue {
+  queue = [];
+  running = 0;
+
+  constructor(concurrency = Infinity) {
+    this.concurrency = concurrency;
+  }
+
+  add(func) {
+    return new Promise((resolve, reject) => {
+      this.queue.push(async () => {
+	this.running++;
+	try {
+	  resolve(await func());
+	} finally {
+	  this.running--;
+	  this.run();
+	}
+      });
+      this.run();
+    });
+  }
+
+  run() {
+    while (this.running < this.concurrency && this.queue.length > 0) {
+      this.queue.shift()();
+    }
+  }
+}
+
+module.exports = {PromiseQueue};