lo icon indicating copy to clipboard operation
lo copied to clipboard

Add task pool

Open Azer0s opened this issue 2 years ago • 16 comments

This is more of a prototype right now but it is already a bit faster than the normal lop.Map implementation.

image

Running on M1 Macbook Pro 2021

Azer0s avatar Mar 13 '22 16:03 Azer0s

Currently I have a return slice and just map the values with a function, I will change this however to let the users manage return data themselves

Azer0s avatar Mar 13 '22 16:03 Azer0s

image

Alright, done. Made it a bit faster even

Azer0s avatar Mar 13 '22 16:03 Azer0s

I have done some benchmarks and will commit them later. As of right now, the task pool implementation is about 4x faster.

Azer0s avatar Mar 13 '22 16:03 Azer0s

Refactored everything according to comments. Shall I add some doc in README? @samber

Azer0s avatar Mar 13 '22 22:03 Azer0s

Yes please!

Can you update the benchmark at the bottom of README?

samber avatar Mar 13 '22 22:03 samber

@samber Done. Could you look through the documentation I added?

Azer0s avatar Mar 14 '22 10:03 Azer0s

@samber I added another helper function called With. This is useful for the situations described in the readme and for some situations with the taskpool.

Azer0s avatar Mar 14 '22 10:03 Azer0s

please merge @samber

Azer0s avatar Mar 27 '22 21:03 Azer0s

Good job. But maybe the API that implemented with method With() is a little complex to use?

I think expandable optional parameters might be more suitable:

func Map[T any, R any](collection []T, iteratee func(T, int) R, options ...*ParallelOption) []R {
}

// Usage Cases:

// set max concurrency count with option
parallel.Map(collection, callback, parallel.Option().Concurrency(20))

// normal calling
parallel.Map(collection, callback)

// expandable to more options in future
options := parallel.Option()
options.Concurrency(10)
options.Timeout(2 * time.Second)
options.Retries(3)
parallel.Map(collection, callback, options)

By the way, this design with optional parameters was well-practiced in other language communities:

Node.js p-map

await pMap(array, callback, { concurrency: 20 })

Node.js bluebird

await Bluebird.map(array, callback, { concurrency: 20 })

Node.js prray

await Prray.from(array).mapAsync(callback, { concurrency: 20 })

Bin-Huang avatar Mar 28 '22 03:03 Bin-Huang

@Bin-Huang Ok I found a solution that imo is a bit more elegant. Kind of a compromise between your and my solution.

Could you take a look? @Bin-Huang @samber

Azer0s avatar Mar 28 '22 14:03 Azer0s

@Azer0s @samber I also wrote some code this weekend, maybe we can share more ideas on the code.

https://github.com/samber/lo/pull/81

Bin-Huang avatar Mar 29 '22 08:03 Bin-Huang

I think this PR is very useful, are we planing to merge it? 😂

yujiachen-y avatar May 21 '23 15:05 yujiachen-y

Would love to see this feature. Bit of a blocker to have unlimited goroutines getting spun up when using parallel functions.

brettmorien avatar Aug 25 '23 23:08 brettmorien

These methods are very useful, when those are be merged ?

kcmvp avatar May 02 '24 03:05 kcmvp

@Azer0s thank your great contribution very much! I like these feature as well. Could please resolve the conflicts and merge it ? @samber

kcmvp avatar May 06 '24 01:05 kcmvp