Some ops seem exceedingly slow on Apple M2

[karalabe]

I wrote an integration today for Geth for both the C and the Go libs. ethereum/go-ethereum#27155

Interestingly, blob to commitmets and proof generations with this lib seem to be 5-6x slower than the pure Go one, which seems be mean that something goes very wrong in compiling the C code: ethereum/go-ethereum#27155 (comment)

Any idea on how to investigate this?

[asn-d6]

This might be an issue with the assembly, but in general c-kzg is not really optimized for commitment generation and proof generation. That's because:

• c-kzg does not do multithreading in the MultiScalar Multiplication, which IIRC go-kzg does. The MSM is the expensive part here so this makes a big difference.
• We did not put much effort into optimizing commitment/proof creation. That was both because of lack of time and also because it's generally slightly harder to do optimizations in C. go-kzg does a bunch of smart caching to avoid expensive divisions etc.
• We also sorta assumed that rollups and anyone who cared about commitment/proof creation would use go-kzg.

[jtraglia]

Hey @karalabe. Yes, we expect Go-KZG-4844 to be faster at those things because they are parallelized. If you run the benchmarks with one core, C-KZG should be faster. I mentioned this at the bottom of my comment here.

To be clear, run GOMAXPROCS=1 go test -bench=. in Go-KZG-4844.

[karalabe]

Well, that's quite a bad practice to do IMO. Concurrency should ideally be done at a higher level, not an an individual crypto peration level that takes millliseconds to complete.

[karalabe]

Do you know where that parallelization code is? Can't seem to find it

[jtraglia]

So these functions are in the prove.go file. I don't think it's explicitly parallelized (like with the go keyword); I think it's just a side-effect of Go executing each function call in a different thread. I don't think it should behave this way by default either.

[asn-d6]

Do you know where that parallelization code is? Can't seem to find it

https://github.com/ConsenSys/gnark-crypto/blob/master/ecc/bls12-381/multiexp.go#L120

[karalabe]

Go doesn’t do magic execution. If there’s no ‘go’ keyword, there’s no parallelization. If gomaxprocs makes a difference, the somewhere deep there are goroutines.

…

On Tue, 25 Apr 2023 at 00:54, Justin Traglia ***@***.***> wrote: So these functions are in the prove.go <https://github.com/crate-crypto/go-kzg-4844/blob/master/prove.go> file. I don't think it's explicitly parallelized (like with the go keyword); I think it's just a side-effect of Go executing each function call in a different thread. I don't think it should behave this way by default either. — Reply to this email directly, view it on GitHub <#290 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA7UGKHZHEZDEO4FR2EOH3XC3ZANANCNFSM6AAAAAAXKCP2DA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[jtraglia]

Ah yeah, sorry I thought it behaved that way but I'm wrong. I believe @asn-d6 has found the real culprit.

[karalabe]

Will run some benchmarks tomorrow, but my gut feeling is that multi threading at that depth is a performance killer opposed to doing it higher.

…

On Tue, 25 Apr 2023 at 01:05, Justin Traglia ***@***.***> wrote: Ah yeah, sorry I thought it behaved that way but I'm wrong. I believe @asn-d6 <https://github.com/asn-d6> has found the real culprit. — Reply to this email directly, view it on GitHub <#290 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA7UGLXJRNDKKE5XPL3MFTXC32LTANCNFSM6AAAAAAXKCP2DA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[kevaundray]

Well, that's quite a bad practice to do IMO. Concurrency should ideally be done at a higher level, not an an individual crypto peration level that takes millliseconds to complete.

In general I agree -- this algorithm in particular is particularly implemented with parallelism in mind.

There is a configuration that you can set to tell it how many go-routines to use here though I chose to not make it accessible for simplicity and due to the fact that it seemed like it was not needed

[karalabe]

That option is not eexposed via go kzg, but thanks for the pointer, will pick it up on the other repo.