SimonErm/react-native-job-queue

Should concurrency work as throttle instead of hard batches?

vicary opened this issue · 11 comments

vicary commented

Most of the time people think the term concurrency as how many active workers independently consuming the next job when they become available. i.e. added to queue or finished a job

But the await Promise.all(processingJobs); in Queue.ts:189 essentially makes it hard batches of the slowest job in each and every batch.

Something like promise-throttle would help.

What do you think?

SimonErm commented

Thank you for this Idea :). I will take a look at it and test an implementation with promise-throttle on a feature branch.

  • ❤️1
vicary commented

Thanks for the prompt reply, would love to see your result. If you think promise-throttle or p-queue has too much overlap with your own code, p-limit is also a great lightweight choice.

I may fork after this for my own features, if you're interested in the things below I can make a separate PR.

  1. Add web support via IndexedDB
  2. Expose jobStore as an option (need a common interface though)
  3. Wraps workers with comlink and react-native-threads
SimonErm commented

I took a look at p-limit and it seems to be a simple implementation, which could be done directly in my queue. But i am not sure if we would run into raceconditions again because of the timeouts. Furthermore, in the current implementaion the concurrency limits the jobs per worker and not the queue. I will try to do some experiments at the weekend.

  1. Add web support via IndexedDB

would be great :)

  1. Expose jobStore as an option (need a common interface though)

I am not sure if i get you right.
Maybe the feature you want to add by using jobStore directly can be added to the queue instead.
I think the possibility to manipulate the jobStore directly could add sideeffects. A possible solution would be a seperate jobStore instance, but i don't know how to handle this with NativeModules.

  1. Wraps workers with comlink and react-native-threads

I would like to keep dependencies as low as possible. But i am thinking about providing an API so that everyone can decide how to execute the workers and embed them in react-native-threads, react-native-workers or something similar if wanted.

vicary commented

Item 2 is mostly for mocking in unit tests, it also opens up a possibility for you to further lighten the dep. tree by spinning off your Native Module into standalone package (it's your call though). It is kind of in conjunction with the API you mentioned in item 3.

SimonErm commented

Sounds good :).
I've implemented a first promise-throttle approach on #14 wich seems to work. Only when the executor throws it seems like i am missing an await but i don't know where.

SimonErm commented

the missing await was just an error in my testcase, so i think it's working.

jladuval commented

Hey folks, need a hand with this? I just discovered this library and I'm pretty sure it's the only maintained React native queue library! Awesome work @SimonErm!

SimonErm commented

Thank you @jladuval.
I think the PR is ready to merge, but i wanted to wait for feedback first, since i don't use this feature by myself.
If you want to implement a new feature or to improve the library feel free to ask or do a PR :-)

vicary commented

I think the PR is good enough, my app is running 2-5 tasks every 30 mins so it's working great, haven't done any real stress test though.

hannojg commented

Also working on my side