2

I've been giving some thought at what could be the best way to deal with a batch of heavy operations with Javascript and I came up with the following:

const results: Promise<void>[] = [];

// Start all pieces of work concurrently
for (const record of await getData()) {
  results.push(doSomeHeavyWork(records));
}

// Collect the results
for (const result of results) {
  // Isolate the failures so one doesn't break another.
  try {
    await result;
  } catch (error) {
    console.log(JSON.stringify(error));
  }
}

My understanding is that the above snippet takes as long as the longest operation, and that's as good as it's going to get AFAIK. However, is there a better or more idiomatic way of going about this problem?

I'm not really looking necessarily at node here. This could be node, Deno or browser code.

Javier García Manzano
  • 1,024
  • 2
  • 12
  • 25
  • In browser code you can use `setTimeout(myTimeConsumingFunction, 0)` which will run the function in a separate browser thread. – Amila Senadheera Dec 28 '21 at 14:54
  • 1
    The event loop doesn't *make* anything concurrent. What does `doSomeHeavyWork` do? Is it actually asynchronous work? – Bergi Dec 28 '21 at 15:05
  • 1
    Assuming `doSomeHeavyWork` returns a promise that sometimes rejects, this code is actually broken and as it may lead to unhandled promise rejections crashing your process. See [here](https://stackoverflow.com/questions/46889290/waiting-for-more-than-one-concurrent-await-operation) and [there](https://stackoverflow.com/questions/45285129/any-difference-between-await-promise-all-and-multiple-await). Use `Promise.all` instead! – Bergi Dec 28 '21 at 15:08
  • I suppose you could implement Web Workers..they run without interfering with the UI thread. Documentation: https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Using_web_workers – sandeep.kgp Dec 28 '21 at 15:23
  • 1
    Is the doSomeHeavyWork doing a lot of IO? If so use promises makes sense. If you are actually doing a lot of CPU work then what you are doing won't help because everything is still running the same the thread. Instead you need to look at using worker threads. – matt helliwell Dec 28 '21 at 16:02
  • It's also important to distinguish between the reasons for long-running async promises (e.g. computationally-expensive tasks vs ones which depend on an inexpensive, but latent result like receiving a network response). – jsejcksn Dec 28 '21 at 19:36
  • Thanks all for the valuable feedback! First off, let's assume `dosomeHeavyWork` does some IO and some CPU, mostly IO. Second off, I understand the benefits of `Promise.all`, but the main point is that I want to wait for *all* the tasks to finish, and handle each resolution/rejection separately. That's the usecase I want to focus on. How would you go about it? – Javier García Manzano Dec 29 '21 at 21:31
  • @Bergi I totally get your point, but `Promise.all` doesn't cut it for me. What would other alternatives be? – Javier García Manzano Dec 29 '21 at 21:32
  • @JavierGarcíaManzano `Promise.all(data.map(record => doSomeHeavyWork(records).catch(…))` *does* cut it for you. Or use `Promise.allSettled` if you don't want to get error logs early. – Bergi Dec 29 '21 at 21:34
  • @Bergi aha, gotcha. I just realised. So the trick here is to actually add a `catch` call on the promises so they don't "blow up". Am I right to understand that by adding that `catch` call the call to `Promise.all` then waits for all of the results? Assuming that `Promise.allSettled` gets me to the same place, it does feel like it expresses the intent better. – Javier García Manzano Dec 29 '21 at 21:40
  • 1
    @JavierGarcíaManzano Doing `Promise.all(data.map(doSomeHeavyWork)).catch(…)` waits for all the results, but fails altogether if any one fails. If you put `.catch()` on each `doSomeHeavyWork` call individually, it will also wait for all the results, but result in `undefined` (or whatever your `catch` callback returned) for those that failed. `Promise.allSettled` will also wait for all the results, but let you handle (or log) errors only once everything has finished, not as soon as the error occurs. – Bergi Dec 29 '21 at 21:50

1 Answers1

1

In your code, just pushing the promises inside the array won't start a concurrency work, but the way you will resolve them that can be made in a concurrency way. In this for loop, each item of the results array will execute synchronously one after another without any concurrency execution and the performance will be very low.

The javascript provides a way to achieve this "concurrency" execution, using the all static method of the Promise native class:

const results: Promise<void>[] = [];

// Start all pieces of work concurrently
for (const record of await getData()) {
  results.push(doSomeHeavyWork(records));
}

// Without handling errors will be just like this
await Promise.all(results);

In the above example, all promises will be resolved, but if one fails all of them will fail too (consider see more about allSettled, if this is not what you want). To handle each promise error, you can just attach a catch block to each promise inside the array:

const results: Promise<void>[] = [];

// Start all pieces of work concurrently
for (const record of await getData()) {
  results.push(doSomeHeavyWork(records));
}

// Handling errors will be just like this
await Promise.all(results.map(r => r.catch(error => console.log(JSON.stringify(error)))));

Note: concurrency is underquoted because nodeJS is a monothread language, so this concurrency does not happen in different threads.

VINICIUS DA CRUZ SORANÇO
  • 79
  • 6
  • "*In this `for` loop, each item of the `results` array will execute synchronously one after another*" - no. As you mention, `results` is holding promises, so nothing does "execute" when you `await` them. – Bergi Dec 28 '21 at 15:53
  • I mean the second `for` loop, where he is "awaiting" the `result` (item of the `results` array). I may have used wrong words to explain, but each promise will be awaited, and the loop just goes to the next promise when this has already been resolved or rejected. @Bergi – VINICIUS DA CRUZ SORANÇO Dec 28 '21 at 18:27
  • Yes, but there's no slowdow through that, "*performance will be very low*" is wrong. All the `doSomeHeavyWork` execution starts in the first loop already, there is no performance difference between `Promise.all`(`settled`) and the original code. Just a correctness one - `Promise.all` doesn't cause unhandled rejection crashes (and it's also simpler code). – Bergi Dec 28 '21 at 19:14