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

Reduce number of (re)allocations #17

Closed
Nashtare opened this issue Feb 1, 2024 · 7 comments
Closed

Reduce number of (re)allocations #17

Nashtare opened this issue Feb 1, 2024 · 7 comments
Assignees
Labels
crate: trace_decoder Anything related to the trace_decoder crate.

Comments

@Nashtare
Copy link
Collaborator

Nashtare commented Feb 1, 2024

There are several allocations / reallocations across the decoder that could probably be alleviated with some refactoring.

To be tackled after #19 probably...

@dvdplm
Copy link
Contributor

dvdplm commented Feb 10, 2024

@Nashtare Can you elaborate a bit on what kind of allocation strategy you would like to see here, or point to examples in the code where you'd like to see refactorings? As it stands the description is a bit vague.

@Nashtare
Copy link
Collaborator Author

There are a few TODOs / collect that could be modified, though I mostly put it as a tracking ticket, following an offline discussion with @BGluth

@BGluth
Copy link
Contributor

BGluth commented Feb 10, 2024

Yeah just to add, there are a handful of unnecessary heap allocations (IIRC mostly to deal with some lifetime issues), but I would expect only extremely negligible performance improvement after dealing with all of them. It might be more to just make the codebase cleaner, since we spend almost all of our time inside plonky2 generating proofs.

@Nashtare
Copy link
Collaborator Author

Yeah I also think it's nice to reduce these AMAP, even though they aren't performance critical compared to the prover. I also have a WIP branch for this purpose on the plonky2 side.

@dvdplm
Copy link
Contributor

dvdplm commented Feb 11, 2024

AMAP? You mean ASAP?

@Nashtare
Copy link
Collaborator Author

As much as possible :) But it could also be ASAAMAP combining both :D

@dvdplm
Copy link
Contributor

dvdplm commented Feb 11, 2024

TIL!

@Nashtare Nashtare transferred this issue from 0xPolygonZero/proof-protocol-decoder Feb 13, 2024
@github-project-automation github-project-automation bot moved this to Backlog in Zero EVM Feb 13, 2024
@Nashtare Nashtare added the crate: trace_decoder Anything related to the trace_decoder crate. label Feb 13, 2024
@pgebheim pgebheim added this to the Cleanups and Misc. milestone Mar 11, 2024
@vladimir-trifonov vladimir-trifonov moved this from Backlog to In Progress in Zero EVM Mar 26, 2024
@vladimir-trifonov vladimir-trifonov moved this from In Progress to Ready To Merge in Zero EVM Mar 26, 2024
@vladimir-trifonov vladimir-trifonov moved this from Ready To Merge to Ready to Review in Zero EVM Mar 26, 2024
@vladimir-trifonov vladimir-trifonov moved this from Ready to Review to Done in Zero EVM Apr 8, 2024
@Nashtare Nashtare modified the milestones: Misc., Performance Tuning Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate: trace_decoder Anything related to the trace_decoder crate.
Projects
Status: Done
Development

No branches or pull requests

5 participants