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

Proposal: Represent Uint128 as two uint64 instead of big.Int #39

Open
Fryuni opened this issue Jul 24, 2022 · 2 comments
Open

Proposal: Represent Uint128 as two uint64 instead of big.Int #39

Fryuni opened this issue Jul 24, 2022 · 2 comments

Comments

@Fryuni
Copy link

Fryuni commented Jul 24, 2022

A big.Int value has 32 bytes in size

  • 1 byte for a bool
  • 8 bytes for a slice pointer (one word)
  • 8 bytes for the slice length (one word)
  • 8 bytes for the slice capacity (one word)
  • 7 padding bytes

The number that it is holding only has 16 bytes, so using a big.Int adds an extra allocation, indirection and triples the memory use.

I propose the type Uint128 to be changed to:

type Uint128 struct { High, Low uint64 }

There are libraries in the community that define a Uint128 with its operations. I don't think there is a need for any operations here, so we could have the serialization and deserialization hand-coded here (using the binary package). If we don't want to deal with those operations we could depend on one such library.

For consumers that already have a big.Int we could provide a Uint128FromBig(*big.Int) Uint128 function.

This would be a braking change on the API, but that is ok by the status on the README.

@oschwald
Copy link
Member

I am curious what your use case for the uint128 data type is. Currently, we don't actually use it and it hasn't received much thought. big.Int was chosen as that is what github.com/oschwald/maxminddb-golang](https://github.com/oschwald/maxminddb-golang) uses for deserialization. If this change is made, it is likely that additional changes will need to be made for loading an existing database containing the values.

When I wrote the reader, I was hoping that something would eventually happen with golang/go#9455, but it has been almost 8 years without movement.

@Fryuni
Copy link
Author

Fryuni commented Jul 25, 2022

Currently just for including an IPv6 in the data (for custom aliasing). It is not a huge deal since it is only a few hundred of those, but using a pointer type larger than the data it points to felt very odd to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants