Parallelize SHPLONK multi-open prover

[jonathanpwang]

The main commit d6f562a basically just changes two iterators to par_iter. On some circuit I tested this sped up the SHPLONK multi-open proving time at the end of create_proof from 12s to 4s.

The other commits are optional and only give minor performance improvements. I used BTreeSet for a slightly more efficient/cleaner implementation of getting the distinct rotation sets. Then the last commit (which can be reverted if necessary) adds Send + Sync to some traits for further parallelization.

[jonathanpwang]

Using HashSet instead of BTreeSet gives some more microscopic improvements, but I didn't want to impose more trait conditions so I didn't bother.

[CPerezz]

LGTM!! Good work with this @jonathanpwang 🥳

[han0110]

I think this will make the order of rotation_sets depend on the opening point, which is making the code generated verifier much more difficult to be implement (imagine we need to sort the points in circuit or in evm).

The test below is trying to make sure the order doesn't depend on the opening point.

halo2/halo2_proofs/src/poly/kzg/multiopen/shplonk.rs

Lines 236 to 255 in 6a46bc9

	proptest! {
	#[test]
	fn test_intermediate_sets(
	(queries_1, queries_2) in compare_queries(8, 8, 16)
	) {
	let IntermediateSets { rotation_sets, .. } = construct_intermediate_sets(queries_1);
	let commitment_sets = rotation_sets.iter().map(|data|
	data.commitments.iter().map(Commitment::get).collect::<Vec<_>>()
	).collect::<Vec<_>>();
	// It shouldn't matter what the point or eval values are; we should get
	// the same exact point set indices and point indices again.
	let IntermediateSets { rotation_sets: new_rotation_sets, .. } = construct_intermediate_sets(queries_2);
	let new_commitment_sets = new_rotation_sets.iter().map(|data|
	data.commitments.iter().map(Commitment::get).collect::<Vec<_>>()
	).collect::<Vec<_>>();
	assert_eq!(commitment_sets, new_commitment_sets);
	}
	}

And it's actually failed with this PR.

One easy approach to make this PR adoptable is to additionally store Rotation in Query, so that we can gain the efficiency without make the order depend on the opening point.

[CPerezz]

One easy approach to make this PR adoptable is to additionally store Rotation in Query, so that we can gain the efficiency without make the order depend on the opening point.

Nice catch @han0110 I totally missed this. 👍

[jonathanpwang]

Good catch @han0110 !
My understanding is we need the collection of rotation sets to stay in the same order, but the ordering within each rotation set doesn't matter, since the verifier just computes the vanishing polynomial.
I think I fixed it just by not using the second BTreeMap (which was probably overkill anyways). It at least passes the prop test.
Can you take a look again?

[han0110]

The commit makes sense and looks good! As long as the commitments order is static, the verifier doesn't need to care about the opening point's order.

Nice improvement!

[jonathanpwang]

@kilic can you review this when you get a chance?