You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@datasketches.apache.org by "Fengzdadi (via GitHub)" <gi...@apache.org> on 2026/01/02 01:02:43 UTC
[I] [HLL] Question: Optimization of byte-to-int conversion in coupon_list.go (TODO at line 193) [datasketches-go]
Fengzdadi opened a new issue, #86:
URL: https://github.com/apache/datasketches-go/issues/86
### Description
I noticed a TODO comment at `hll/coupon_list.go:193`:
```go
// TODO there must be a more efficient to reinterpret the byte array as an int array
for it := 0; it < couponCount; it++ {
list.couponIntArr[it] = int(binary.LittleEndian.Uint32(byteArray[listIntArrStart+it*4 : listIntArrStart+it*4+4]))
}
```
I'd like to discuss potential optimizations for this code path.
### Research
I investigated how other DataSketches implementations handle this:
| Language | Approach |
|----------|----------|
| **Java** | `Memory.getIntArray()` - bulk copy using `Unsafe.copyMemory()` |
| **C++** | `std::memcpy()` - direct memory copy |
| **Go (current)** | Loop with `binary.LittleEndian.Uint32()` |
### Benchmark Results
I ran some benchmarks comparing different Go approaches:
| Method | 8 items | 32 items | 128 items | 1024 items |
|--------|---------|----------|-----------|------------|
| **Current (loop)** | 27.6 ns | 75.4 ns | 270 ns | 2153 ns |
| **binary.Read** | 101 ns | 237 ns | 692 ns | 5112 ns |
| **Precompute offset** | 26.8 ns | 69.9 ns | 253 ns | 2000 ns |
| **Loop unrolling** | 27.6 ns | 69.7 ns | 248 ns | 1998 ns |
**Key findings:**
1. `binary.Read` is actually **slower** due to reflection and extra allocations
2. Pre-computing the offset provides ~7% improvement
3. The current implementation is already quite efficient
### Proposed Changes
**Option A: Minor optimization (pre-compute offset)**
```go
base := listIntArrStart
for it := 0; it < couponCount; it++ {
list.couponIntArr[it] = int(binary.LittleEndian.Uint32(byteArray[base:]))
base += 4
}
```
- ~7% performance improvement
- Cleaner code
- Remove or update TODO comment
**Option B: Use `unsafe` package**
```go
src := byteArray[listIntArrStart : listIntArrStart+couponCount*4]
int32Slice := unsafe.Slice((*int32)(unsafe.Pointer(&src[0])), couponCount)
```
- Near-zero overhead (similar to C++ memcpy)
- Introduces `unsafe` dependency
- May have portability concerns
**Option C: Keep current implementation**
The current implementation is already quite efficient. Perhaps just update the TODO to:
```go
// Note: Alternative approaches (binary.Read, unsafe) were benchmarked but did not
// provide significant improvements. The current loop-based approach is optimal for Go.
```
### Questions for Maintainers
1. Would you accept Option A (minor optimization) as a PR?
2. Is the project open to using `unsafe` for performance-critical paths?
3. Should the TODO be updated/removed to reflect that the current approach is adequate?
### Environment
- Go version: 1.25.5
- OS: Windows
- CPU: Intel Core i7-10700K
---
I'm happy to submit a PR for any of these options based on your feedback!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] [HLL] Question: Optimization of byte-to-int conversion in coupon_list.go (TODO at line 193) [datasketches-go]
Posted by "freakyzoidberg (via GitHub)" <gi...@apache.org>.
freakyzoidberg commented on issue #86:
URL: https://github.com/apache/datasketches-go/issues/86#issuecomment-3705028072
We can start with A.
HLL already uses unsafe in some places - we'll need to be careful obviously but it may be acceptable depending of the portability impact.
(the DataSketches Go and other libs already have some portability limitation especially around Endianess)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] [HLL] Question: Optimization of byte-to-int conversion in coupon_list.go (TODO at line 193) [datasketches-go]
Posted by "freakyzoidberg (via GitHub)" <gi...@apache.org>.
freakyzoidberg closed issue #86: [HLL] Question: Optimization of byte-to-int conversion in coupon_list.go (TODO at line 193)
URL: https://github.com/apache/datasketches-go/issues/86
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] [HLL] Question: Optimization of byte-to-int conversion in coupon_list.go (TODO at line 193) [datasketches-go]
Posted by "proost (via GitHub)" <gi...@apache.org>.
proost commented on issue #86:
URL: https://github.com/apache/datasketches-go/issues/86#issuecomment-3704358242
Option A is enough.
As you said, portability issue happen.
1. endianness
2. memory alignment.
most modern CPU architectures works fine. but some old architecture happen weird things.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] [HLL] Question: Optimization of byte-to-int conversion in coupon_list.go (TODO at line 193) [datasketches-go]
Posted by "Fengzdadi (via GitHub)" <gi...@apache.org>.
Fengzdadi commented on issue #86:
URL: https://github.com/apache/datasketches-go/issues/86#issuecomment-3721581118
Thanks for the heads-up @leerho!
I'll make sure to reference ds-java 9.0.0+ when looking at the Java implementation. Good to know about the Panama FFM MemorySegment migration – that's exciting for performance!
For the Go implementation, I'm currently working on the reservoir sampling serialization (#94). I'll keep the latest Java version in mind for any future reference.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] [HLL] Question: Optimization of byte-to-int conversion in coupon_list.go (TODO at line 193) [datasketches-go]
Posted by "leerho (via GitHub)" <gi...@apache.org>.
leerho commented on issue #86:
URL: https://github.com/apache/datasketches-go/issues/86#issuecomment-3716916540
If you are looking at a datasketches-java implementation and see a reference to Memory.getIntArray(...) you are looking at the wrong version of ds-java. Please use the Java 25 version, ds-java 9.0.0, which eliminated the use of the now deprecated Memory component. It has switched all use of Memory to the new "Panama" FFM MemorySegment, which is much faster. :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] [HLL] Question: Optimization of byte-to-int conversion in coupon_list.go (TODO at line 193) [datasketches-go]
Posted by "Fengzdadi (via GitHub)" <gi...@apache.org>.
Fengzdadi commented on issue #86:
URL: https://github.com/apache/datasketches-go/issues/86#issuecomment-3706447645
Thank you for the feedback! I've submitted PR #87 implementing Option A (pre-compute offset).
I also learned a lot from the discussion about endianness and cross-platform compatibility in the Rust issue - it's a great insight into the project's design philosophy.
I'm a newcomer to open source contributions. I noticed the Rust implementation has been gaining momentum with some great contributors recently, which inspired me to try contributing to the Go version since it's the language I'm most familiar with. Looking forward to learning more and contributing further! 😄
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org