Seemingly needless clearing of array values

[holiman]

Looking at the code here: https://github.com/Consensys/gnark-crypto/blob/master/ecc/bls12-381/multiexp_jacobian.go#L108

func processChunkG2Jacobian[B ibg2JacExtended](chunk uint64,
	chRes chan<- g2JacExtended,
	c uint64,
	points []G2Affine,
	digits []uint16,
	sem chan struct{}) {

	if sem != nil {
		// if we are limited, wait for a token in the semaphore
		<-sem
	}

	var buckets B
	for i := 0; i < len(buckets); i++ {
		buckets[i].setInfinity()
	}

The code waits for a go-ahead, to start processing the items. The wait, I presume, is to ensure that the worker doesn't interfere with other threads, or maybe that the inputs are not ready yet.

However, the local variable buckets is not shared, and thus it should be perfectly ok to do the buckets[i].setInfinity()-loop before waiting for the <-sem.

Looking into it a bit deeper, however, I see that the buckets is a newly stack-allocated array of g2JacExtended. For example, it might be [32]g2JacExtended.

The g2JacExtended is:

type g2JacExtended struct {
	X, Y, ZZ, ZZZ fptower.E2
}

and the setInfinity() is

func (p *G2Affine) setInfinity() *G2Affine {
	p.X.SetZero()
	p.Y.SetZero()
	return p
}

As far as I can tell, the setInfinity -> SetZero methods, in the end, boil down to zeroing uint64-arrays. This is not necessary in golang, newly allocated arrays (heap or stack) are always zeroed.

So afaict the buckets[i].setInfinity()-loop can just be dropped. Perhaps I am missing something?

[holiman]

Ah, it seems that g1JacExtended are already at inifinity (zeroed) at construction, whereas g2JacExtended are not at infinity when zero. So e.g. this is actually required:

	var runningSum, total g2JacExtended
	runningSum.setInfinity()
	total.setInfinity()

(but the buckets should still be fine from the get-go)

[ivokub]

cc. @gbotrel, but it seems right that for G1 methods we can avoid zeroing the buckets.

[gbotrel]

yes indeed, we can avoid zeroing the buckets twice with the affine msm. I suspect the code is here because some previous iterations had the buckets in extended jacobian coordinates; don't think it's going to be visible on benchmarks though.