PriorityQueue implementation for jest-worker

[MichaReiser]

Summary

The current jest-worker implementation processes tasks in FIFO ordering. This assumes that all tasks are equally important which isn't always given.

This PR extracts a TaskQueue interface, extracts the existing implementation as FifoQueue, adds a new PriorityQueue implementations and allows customizing the used queue implementation when creating the JestWorker.

A TaskQueue offers two operations:

• enqueue(workerId: ?number): Add a new task to the queue of the specified worker or for all workers if the workerId is absent
• dequeue(workerId: number): Return the next task to process for the specified worker id.

The PriorityQueue uses a MinHeap that has log(n) complexity for enqueue and O(1) complexity for the dequeue implementation. The implementation uses a shared and n worker specific queues to support sticky workers. The implementation returns the tasks in the ordering according to their priority. Tasks with equal priority are returned in any order except that sticky-tasks are returned before tasks queued in the shared queue.

This diff extracts the current queue implementation into the FifoQueue class and refactors the implementation to reduce the memory footprint. The current implementation added non-worker specific tasks to the queue of each worker. The result is that there are n * tasks queue entries where n is the number of workers for shared tasks. Furthermore, dequeuing required to iterate through all tasks from the top until a task that hasn't been processed is found. The refactored implementation uses a shared queue and a separate queue for each worker. Shared tasks are only enqueued in the shared queue. Each task can only be part of one queue making it redundant to find the first non-processed task during dequeue.

Test plan

Unit tests for the new queue implementations. Existing integration tests pass with the refactored FIFO queue implementation.

Empty

FIFO Current

---------------------------------------------------------------------------
total worker-farm: { wFGT: 85202.2141059637, wFPT: 85074.46953901649 }
total jest-worker: { jWGT: 24398.66391697526, jWPT: 24248.10139900446 }
---------------------------------------------------------------------------
% improvement over 100000 calls (global time): 71.3638146930885
% improvement over 100000 calls (processing time): 71.49779301546641

FIFO Refactored

---------------------------------------------------------------------------
total worker-farm: { wFGT: 88682.86321604252, wFPT: 88576.52663603425 }
total jest-worker: { jWGT: 24277.916941016912, jWPT: 24127.625441014767 }
---------------------------------------------------------------------------
% improvement over 100000 calls (global time): 72.62389140292767
% improvement over 100000 calls (processing time): 72.76070042782725

loadTest

FIFO Current

total worker-farm: { wFGT: 181426.06709498167, wFPT: 181314.26368799806 }
total jest-worker: { jWGT: 153297.50806596875, jWPT: 153158.72005698085 }
---------------------------------------------------------------------------
% improvement over 100000 calls (global time): 15.504144183584613
% improvement over 100000 calls (processing time): 15.528587248638475

FIFO Refactored

---------------------------------------------------------------------------
total worker-farm: { wFGT: 177797.00552800298, wFPT: 177702.07569798827 }
total jest-worker: { jWGT: 146847.8964600265, jWPT: 146713.39515596628 }
---------------------------------------------------------------------------
% improvement over 100000 calls (global time): 17.406991178544903
% improvement over 100000 calls (processing time): 17.438558565116864

[SimenB]

@MichaReiser @mjesun is this waiting for me? 😅

[MichaReiser]

@SimenB would be great if you could take a look at it to get someone else's opinion on the changes and if this is something that would be beneficial for jest-worker.

[SimenB]

I assumed this was some FB thing that you needed and it'd go through without me having to do anything except publishing 😅 From skimming I think this looks good, especially as there's no change in default behavior. And the motivation for the new feature seems sound to me 👍 And of course the improved benchmarks you posted are awesome

[SimenB]

Suggested change

	priorities.sort((a, b) => a - b);
	priorities.sort();

[MichaReiser]

The override is required because JavaScript does by default a lexicographical sort.

The sort() method sorts the elements of an array in place and returns the sorted array. The default sort order is ascending, built upon converting the elements into strings, then comparing their sequences of UTF-16 code units values. MDN

[10, 3, 4, 8, 2, 9, 7, 1, 2, 6, 5].sort()
Array(11) [ 1, 10, 2, 2, 3, 4, 5, 6, 7, 8, … ]

[SimenB]

JS strikes again 😀

[MichaReiser]

Thanks @SimenB for your thoughtful review!

I assumed this was some FB thing that you needed and it'd go through without me having to do anything except publishing

It did start as part of some work on Metro but then turned out that switching to a priority queue does not improve the performance as much as we hoped for. I then decided to clean up the implementation because in my spare time because I believed the separation of concerns (extracting the task queue) is worth it and a priority queue might come in handy for someone else (or we might need it in the end).

[MichaReiser]

I resolved the conflicts of this PR and added the changelog entry. Should now be ready to merge.

[SimenB]

Thanks!

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.