Optimize canonical linear relation constraint processing #98
Conversation
src/linear_relation/canonical.rs
Outdated
|
|
||
| /// Access the group elements associated with the image (i.e. left-hand side), panicking if any | ||
| /// of the image variables are unassigned in the group mkap. | ||
| pub(crate) fn image_elems(&self) -> impl Iterator<Item = G> + use<'_, G> { |
There was a problem hiding this comment.
Sometime we are using _elements, sometimes elems, let's call this elements for consistency
src/linear_relation/canonical.rs
Outdated
| for elem_repr in group_elements_ordered { | ||
| out.extend_from_slice(elem_repr.as_ref()); | ||
| // Dump the group elements. | ||
| for (_, elem) in self.group_elements.iter() { |
There was a problem hiding this comment.
uhm here I'm noticing we're not using our API for serialization.
It's not a big deal, but it'd make the code more consistent + there's batch serialization formulae for elliptic curve implementation (relying on batch affine conversion) we might want to switch to that.
I don't think we need to change this now, but a placeholder would help
There was a problem hiding this comment.
I can go ahead and change that. I didn't notice that when I made the change. This code is also just a lot simpler now. What I realized is that when I first wrote this, the work that is happening in TryFrom now was happening here (IIRC).
I did some profiling and iterated optimization on statement verification using the OONI benchmarks. The following changes result in ~40% reduction in verification time on my laptop.
get_or_create_weighted_group_var.Breaking changes
CanonicalLinearRelation::imagehas a changed type fromVec<G>toVec<GroupVar>.serialization::serialize_elementsnow uses animpl IntoIteratorsignature for its argument. This should not effect any typical usage of this function (when a slice is passed in and the type of thefnis not directly asserted)