From d1ae23c7500359e7039eadca54827c32959cb66f Mon Sep 17 00:00:00 2001
From: Ross Wollman <rwoll@users.noreply.github.com>
Date: Mon, 18 Apr 2022 07:05:38 -0700
Subject: [PATCH] cherry-pick(#13584): avoid premature stop of worker in
 fullyParallel (#13597)

Prior to this change, we were pre-maturely stopping a worker (since it
was deemed redundant under a race condition), and then we immediately
created a new worker with the same hash to finish off the test run. The
worker creation is expensive, so this slowed down the overall test run
time.

See the following for logs of the old code illustrating the extra stops and starts: https://gist.github.com/rwoll/1c592ed9e8f9169274fa972674de6703
---
 packages/playwright-test/src/dispatcher.ts | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/packages/playwright-test/src/dispatcher.ts b/packages/playwright-test/src/dispatcher.ts
index ad676bf0d5354..d7f14241c8d6a 100644
--- a/packages/playwright-test/src/dispatcher.ts
+++ b/packages/playwright-test/src/dispatcher.ts
@@ -43,7 +43,7 @@ type TestData = {
 export class Dispatcher {
   private _workerSlots: { busy: boolean, worker?: Worker }[] = [];
   private _queue: TestGroup[] = [];
-  private _queueHashCount = new Map<string, number>();
+  private _queuedOrRunningHashCount = new Map<string, number>();
   private _finished = new ManualPromise<void>();
   private _isStopped = false;
 
@@ -58,7 +58,7 @@ export class Dispatcher {
     this._reporter = reporter;
     this._queue = testGroups;
     for (const group of testGroups) {
-      this._queueHashCount.set(group.workerHash, 1 + (this._queueHashCount.get(group.workerHash) || 0));
+      this._queuedOrRunningHashCount.set(group.workerHash, 1 + (this._queuedOrRunningHashCount.get(group.workerHash) || 0));
       for (const test of group.tests)
         this._testById.set(test._id, { test, resultByWorkerIndex: new Map() });
     }
@@ -80,7 +80,6 @@ export class Dispatcher {
 
     // 3. Claim both the job and the worker, run the job and release the worker.
     this._queue.shift();
-    this._queueHashCount.set(job.workerHash, this._queueHashCount.get(job.workerHash)! - 1);
     this._workerSlots[index].busy = true;
     await this._startJobInWorker(index, job);
     this._workerSlots[index].busy = false;
@@ -143,7 +142,7 @@ export class Dispatcher {
       if (slot.worker && !slot.worker.didSendStop() && slot.worker.hash() === worker.hash())
         workersWithSameHash++;
     }
-    return workersWithSameHash > this._queueHashCount.get(worker.hash())!;
+    return workersWithSameHash > this._queuedOrRunningHashCount.get(worker.hash())!;
   }
 
   async run() {
@@ -273,6 +272,7 @@ export class Dispatcher {
     worker.on('stepEnd', onStepEnd);
 
     const onDone = (params: DonePayload) => {
+      this._queuedOrRunningHashCount.set(worker.hash(), this._queuedOrRunningHashCount.get(worker.hash())! - 1);
       let remaining = [...remainingByTestId.values()];
 
       // We won't file remaining if:
@@ -379,7 +379,7 @@ export class Dispatcher {
 
       if (remaining.length) {
         this._queue.unshift({ ...testGroup, tests: remaining });
-        this._queueHashCount.set(testGroup.workerHash, this._queueHashCount.get(testGroup.workerHash)! + 1);
+        this._queuedOrRunningHashCount.set(testGroup.workerHash, this._queuedOrRunningHashCount.get(testGroup.workerHash)! + 1);
         // Perhaps we can immediately start the new job if there is a worker available?
         this._scheduleJob();
       }