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

Add option to decode CBOR bignum to interface{} as *big.Int #456

Merged
merged 1 commit into from
Dec 28, 2023

Conversation

fxamacker
Copy link
Owner

@fxamacker fxamacker commented Dec 27, 2023

Closes #444

This PR adds decoding option BigIntDecMode which specifies how to decode CBOR bignum to Go interface{}.

  • BigIntDecodeValue (default) makes CBOR bignum decode to big.Int.
  • BigIntDecodePointer makes CBOR bignum decode to *big.Int.

Default behavior is unchanged to maintain backward compatibility. However, using the new option is recommended as it improves speed, reduces memory use, and is more ergonomic.

Benchmark Comparison

                  │ bench_decode_to_bigint.txt │     bench_decode_to_pbigint.txt      │
                  │           sec/op           │    sec/op     vs base                │
DecodeToBigInt-12                  238.9n ± 8%   196.2n ± 10%  -17.87% (p=0.000 n=20)

                  │ bench_decode_to_bigint.txt │    bench_decode_to_pbigint.txt     │
                  │            B/op            │    B/op     vs base                │
DecodeToBigInt-12                   144.0 ± 0%   112.0 ± 0%  -22.22% (p=0.000 n=20)

                  │ bench_decode_to_bigint.txt │    bench_decode_to_pbigint.txt     │
                  │         allocs/op          │ allocs/op   vs base                │
DecodeToBigInt-12                   5.000 ± 0%   4.000 ± 0%  -20.00% (p=0.000 n=20)

Special Thanks

Many thanks to @extemporalgenome for opening issue #444! 👍

Added decoding option BigIntDecMode which specifies how to decode
CBOR bignum to Go interface{}.

- BigIntDecodeValue (default) makes CBOR bignum decode to big.Int.
- BigIntDecodePointer makes CBOR bignum decode to *big.Int.
@fxamacker fxamacker added the enhancement New feature or request label Dec 27, 2023
@fxamacker fxamacker added this to the v2.6.0 milestone Dec 27, 2023
@fxamacker fxamacker self-assigned this Dec 27, 2023
Copy link
Contributor

@x448 x448 left a comment

Choose a reason for hiding this comment

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

LGTM!

Maybe show benchstat comparison for ns/op and alloc/op between these settings.

@fxamacker
Copy link
Owner Author

@x448 Thanks! I added benchmark comparisons to the PR text.

@fxamacker fxamacker merged commit ef38c6f into master Dec 28, 2023
17 checks passed
@fxamacker fxamacker deleted the fxamacker/add-decode-option-for-bigint branch December 28, 2023 21:18
@extemporalgenome
Copy link

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feature: add option to decode to *big.Int instead of big.Int when decoding CBOR bignum into any
3 participants