Skip to content

Conversation

@jsign
Copy link
Collaborator

@jsign jsign commented Dec 13, 2022

This PR avoids a forced heap allocation since it avoids returning a pointer from a stack variable. This was noticed while looking at profiling information in go-verkle.

It also removes an unnecessary assignment.

Linked to ethereum/go-verkle#314

@kevaundray
Copy link
Contributor

PR looks good to me, removing the vv.Set seems like something we could upstream to gnark-crypto!

Signed-off-by: Ignacio Hagopian <[email protected]>
@jsign jsign marked this pull request as ready for review January 11, 2023 19:44
@jsign
Copy link
Collaborator Author

jsign commented Jan 11, 2023

@kevaundray, this is ready for review (or merge really).
The change with the previous version is only removing the v.Set(...) improvement since this was upstreamed to gnark-crypto.

I've tested the impact of removing this, and despite isn't zero I think we can live with it. So, we can have the benefit of one less "custom" change to generated code for the future when we try to reorganize ourselves.

@kevaundray kevaundray merged commit 98736f2 into crate-crypto:master Jan 12, 2023
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