Skip to content

Add benchmarks for Quantity operations#1477

Closed
theycallmeaabie wants to merge 1 commit into
hyperledger-labs:mainfrom
theycallmeaabie:feat/quantity-benchmarks
Closed

Add benchmarks for Quantity operations#1477
theycallmeaabie wants to merge 1 commit into
hyperledger-labs:mainfrom
theycallmeaabie:feat/quantity-benchmarks

Conversation

@theycallmeaabie
Copy link
Copy Markdown
Contributor

Summary

  • Add benchmarks for BigQuantity and UInt64Quantity operations (Add, Sub, Cmp)
  • Add benchmark for ToQuantity parsing

Resolves #1467

Copilot AI review requested due to automatic review settings April 5, 2026 09:40
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds Go benchmarks to track performance of Quantity implementations and parsing over time (per #1467).

Changes:

  • Adds benchmarks for BigQuantity operations: Add, Sub, Cmp
  • Adds benchmarks for UInt64Quantity operations: Add, Sub, Cmp
  • Adds a benchmark for ToQuantity parsing

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread token/token/quantity_test.go
Comment thread token/token/quantity_test.go
Comment thread token/token/quantity_test.go
Comment thread token/token/quantity_test.go
Comment thread token/token/quantity_test.go
Comment thread token/token/quantity_test.go
Comment thread token/token/quantity_test.go
@adecaro
Copy link
Copy Markdown
Contributor

adecaro commented Apr 10, 2026

Hi @theycallmeaabie , please, resolve the conflicts 🙏

@adecaro adecaro requested review from adecaro April 10, 2026 08:04
@adecaro adecaro self-assigned this Apr 10, 2026
@adecaro adecaro modified the milestone: Q2/26 Apr 10, 2026
@theycallmeaabie theycallmeaabie force-pushed the feat/quantity-benchmarks branch from 60657fb to f51e47b Compare April 10, 2026 08:34
@theycallmeaabie
Copy link
Copy Markdown
Contributor Author

Done!

@AkramBitar AkramBitar linked an issue Apr 15, 2026 that may be closed by this pull request
@AkramBitar
Copy link
Copy Markdown
Contributor

@theycallmeaabie, I would like to thank you a lot for submitting this PR.
I opened issue #1553 for this PR.

We will keep issue #1467 to track other sub issues that we have in the list: #1467 (comment)

We will open sub issues under #1467 for each items in the list.

Could you please unlink #1467 from this PR?


func BenchmarkToQuantity(b *testing.B) {
for b.Loop() {
benchResult, _ = token.ToQuantity("0x1234567890abcdef", 64)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please do not ignore errors. Assert on errors:

benchResult, err = token.ToQuantity("0x1234567890abcdef", 64)
require.NoError(t, err)


func BenchmarkUInt64Quantity_Sub(b *testing.B) {
for b.Loop() {
q, _ := token.ToQuantity("1000000000", 64)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please do not ignore errors. Assert on errors:

q, _ := token.ToQuantity("1000000000", 64)
require.NoError(t, err)

for b.Loop() {
r = q.Cmp(one)
}
_ = r
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See copilot error.


func BenchmarkBigQuantity_Sub(b *testing.B) {
for b.Loop() {
q, _ := token.ToQuantity("1000000000", 128)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you please assert on errors and not ignoring it?

This comment is relevant for all other places in these methods.

@AkramBitar
Copy link
Copy Markdown
Contributor

Hi @theycallmeaabie, any news on that PR?

@AkramBitar
Copy link
Copy Markdown
Contributor

Hi @theycallmeaabie, thanks a lot for you efforts on that PR.
Did you have the chance to look at my above comments? Please let me know if you have free cycles to procced with this one.

@adecaro
Copy link
Copy Markdown
Contributor

adecaro commented May 12, 2026

Hi @theycallmeaabie , I'm closing this one. If you believe you can restart the work, please, open this. Thanks for the understanding 🙏

@adecaro adecaro closed this May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add benchmarks to track performance over time chore(token/token/quantity.go): improvements

4 participants