crate-crypto / go-eth-kzg

Apache License 2.0
29 stars 22 forks source link

Add config parameter to set the number of go routines #45

Closed kevaundray closed 1 year ago

kevaundray commented 1 year ago

closes #44

More context: This sets the number of go routines when committing to a polynomial, ny exposing a way to set the gnark-crypto library parameter.

karalabe commented 1 year ago

This feels a bit wonky because the context is global for the library but concurrency may or may not be wanted for individual operations. Also it's unclear which operations use concurrency and which do not.

kevaundray commented 1 year ago

idual operations. Also it's unclear which operations use concurrency and which do not.

I initially put it on each method, but thought that maybe folks did not want that level of control. Good point about knowing which method uses concurrency or not.

I'll re-work and make a 0.2 version since this is a different API

karalabe commented 1 year ago

Perhaps you could have the current methods default to 1 threads and have a second set that can use concurrency internally? That way it would be obvious which can use it and which cannot, and also you could control it individually?

It does feel a bit weird to spec a second set of methods, but IMHO most people will prefer to use the single threaded version either way and if someone needs the multi one, they can explicitly do so.

kevaundray commented 1 year ago

Perhaps you could have the current methods default to 1 threads and have a second set that can use concurrency internally? That way it would be obvious which can use it and which cannot, and also you could control it individually?

It does feel a bit weird to spec a second set of methods, but IMHO most people will prefer to use the single threaded version either way and if someone needs the multi one, they can explicitly do so.

I made each method take the numGoRoutines parameter explicitly in #46 -- Yeah I'd prefer to keep the API as small as possible, the trade-off here being that the default is not configured automatically.

So we could have something like this:

fn foo(x : int) {
 foo_go(x, 1)
}
fn foo_go(x : int, numGoRoutines : int) {}

And thinking about it abit more, it will be for three methods, so its not tooo bad.

Please let me know what you think of 46 !

kevaundray commented 1 year ago

Gonna merge and ask for forgiveness later :)