nicolas-van / modern-async

A modern JavaScript tooling library for asynchronous operations using async/await, promises and async generators
https://nicolas-van.github.io/modern-async/
MIT License
200 stars 8 forks source link

set priority #22

Open stephen-dahl opened 1 month ago

stephen-dahl commented 1 month ago

I would like to be able to set the priority that functions run at. Queue supports it but the rest of the library doesn't have a way to pass priority through to the queue.

I have nested asyncMaps using the same queue. I want to prioritize the inner map over the outer map so that we dont get a race condition where the queue gets stuck because inner items are queued while outer items are taking all the running slots.

const queue = new Queue(3)
const results = await asyncMap(list1, i1 => {
    return asyncMap(list2, i2 => {
        return process(i1, i2)
      },
      queue),
  queue,
);
nicolas-van commented 1 month ago

Actually you can already do what you want with this kind of code:

const queue = new Queue(3)
const results = await asyncMap(list1, i1 => {
    return asyncMap(list2, i2 => {
        return queue.exec({} => process(i1, i2), 1)
      }, Number.POSITIVE_INFINITY);
}, queue);

And yes, the fact that we can't easily set the priority of tasks in a queue through any of the helper functions of the library is, indeed, a concern to me. I'm planning to include some kind of Queue adapter in the future to allow forwarding another default priority.

It could look something like this:

const queue = new Queue(...)
...
await asyncMap(lst, fct, new QueuePriorityOverrider(queue, 3))

But, in your case, I would advise against using this kind of solution.

You see, even if you alter the priorities of the tasks, it could still lead to potential dead locks. The problem is that your queue will always need to have a concurrency >= the maximum deepness of the nested calls to Queue.exec(). In your example you would need to have a concurrency >= 2 or you would hit a certain dead lock.

Sure, you could manually ensure that maximum deepness in your whole code and make sure your queue has at least that exact amount as concurrency. But later, if you (or potentially some one else, like one of your colleagues) alters that code it will be hard to remember to track that amount. We can even be creative about potential modifications of the code and imagine that some calls to exec are hidden within an if (thus sometimes it would work and sometimes it would deadlock) or that it becomes recursive at one point (thus making it doomed to dead lock one day or another, depending on the quantity of data to handle).

Those are the reasons I would generally advise against any kind of case where a function calls reserving a slot in a Queue tries to block on another function call reserving a slot in the same Queue.

Potential solutions:

One queue per layer:

const queue1 = new Queue(3)
const queue2 = new Queue(3)
const results = await asyncMap(list1, i1 => {
    // pre layer 1 code
    const layer2Results = asyncMap(i1.list2, i2 => {
        // layer 2 code
      }, queue2);
   // post layer 1 code
}, queue1);

Here you're clear about what is the concurrency of each layer. All layer 2 functions will still be executed in a common queue.

Only 1 queue but clear separation of each operation in different slots:

const queue = new Queue(3)
const results = await asyncMap(list1, i1 => {
    const preLayer1CodeResults = queue.exec(() => {
      // pre layer 1 code
    })
    const layer2Results = asyncMap(preLayer1CodeResults.list2, i2 => {
        // layer 2 code
      }, queue);
    const postLayer1CodeResults = queue.exec(() => {
      // post layer 1 code
    })
    return postLayer1CodeResults.result
}, Number.POSITIVE_INFINITY);

Here you still have a common queue, but instead of your example it doesn't need to reserve more than 1 slot at a time and will work smoothly regardless of the concurrency of the queue.