Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf/feature: new serialization format for constraint systems #1119

Merged
merged 9 commits into from
Apr 30, 2024

Conversation

gbotrel
Copy link
Collaborator

@gbotrel gbotrel commented Apr 29, 2024

Description

This PR uses integer compression techniques to serialize some constraint systems parts containing ids, offsets, etc and bypass our current cbor choice. (cbor is still used for the rest).

As a bench, using lzss.DecompressionTestCircuit with params landing at 90M constraints (plonk), we go from 9225Mb to 2238Mb size, plus, we serialize (and most importantly deserialize) way faster:

benchmark                                      old ns/op        new ns/op      delta
BenchmarkSerialization/serialize_cbor-96       54962357901      3086254450     -94.38%
BenchmarkSerialization/serialize_cbor-96       41877637150      3117870148     -92.55%
BenchmarkSerialization/deserialize_cbor-96     100910397720     4267472009     -95.77%
BenchmarkSerialization/deserialize_cbor-96     117815108278     3776636214     -96.79%

@gbotrel gbotrel marked this pull request as draft April 29, 2024 02:57
@gbotrel gbotrel requested review from Tabaie and ivokub April 29, 2024 21:26
@gbotrel gbotrel marked this pull request as ready for review April 29, 2024 21:26
Copy link
Collaborator

@ivokub ivokub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice optimizations! I think in general is good, but in a perfect scenario I would try to figure out if we can work with io.Writer and io.Reader instead of direct byte arrays as imo otherwise we have many copies of very similar structures in memory (constraint system itself, bytes when serializing them and also byte buffer for internal steps a la serializing calldata). But due to applying compression to uints we still cannot get away with all.

And I completely agree that it doesn't make sense for us to include additional compression in the implementation. It should be straightforward to add gzip etc. when the user is interested in it. Otherwise there is too many options (fast or good compression ...)

constraint/marshal.go Show resolved Hide resolved
internal/backend/ioutils/intcomp_test.go Show resolved Hide resolved
@ivokub
Copy link
Collaborator

ivokub commented Apr 30, 2024

Very nice optimizations! I think in general is good, but in a perfect scenario I would try to figure out if we can work with io.Writer and io.Reader instead of direct byte arrays as imo otherwise we have many copies of very similar structures in memory (constraint system itself, bytes when serializing them and also byte buffer for internal steps a la serializing calldata). But due to applying compression to uints we still cannot get away with all.

And I completely agree that it doesn't make sense for us to include additional compression in the implementation. It should be straightforward to add gzip etc. when the user is interested in it. Otherwise there is too many options (fast or good compression ...)

Yep, it makes sense that if we work with []byte then can have parallel processing which is more important than saving a few GB in settings where memory usage is in tens of GBs anyway. Added the test corpus.

Good to merge from my side.

@gbotrel gbotrel merged commit f3c5cf3 into master Apr 30, 2024
7 checks passed
@gbotrel gbotrel deleted the perf/cbor_circuit branch April 30, 2024 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants