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

Optimize memory for Int.BigInt() #17772

Closed
hieuvubk opened this issue Sep 16, 2023 · 3 comments · Fixed by #17803
Closed

Optimize memory for Int.BigInt() #17772

hieuvubk opened this issue Sep 16, 2023 · 3 comments · Fixed by #17803
Labels
C:math Component: Math

Comments

@hieuvubk
Copy link
Contributor

There are many converts or calculations called Int.BigInt(). Even though the Int structure already contains i *big.Int, new pointer memory is still reallocated. So is there any way that we can reuse the buffer?
I tried returning Int.i directly in my local instead of declaring new memory and here are the benchmark results. However, I'm not sure if direct access poses any danger. Love to hear everyone's opinions.

OLD:
BenchmarkBigInt-16      21329467                47.96 ns/op           24 B/op          3 allocs/op

NEW:
BenchmarkBigInt-16      424790271                2.689 ns/op           0 B/op          0 allocs/op
@github-actions github-actions bot added the needs-triage Issue that needs to be triaged label Sep 16, 2023
@alexanderbez
Copy link
Contributor

I think the aim was to avoid mutation of the original Int object. However, we can have two APIs -- one that returns a copy and one that returns the field (allowing mutation).

@hieuvubk
Copy link
Contributor Author

I think the aim was to avoid mutation of the original Int object. However, we can have two APIs -- one that returns a copy and one that returns the field (allowing mutation).

That would be great! Also do u want me to make a pr?

@alexanderbez
Copy link
Contributor

sure!

@tac0turtle tac0turtle added C:math Component: Math and removed needs-triage Issue that needs to be triaged labels Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:math Component: Math
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants