Add little-endian reads for byte arrays #5836
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds little-endian equivalents for all the big-endian read operations on mutable and immutable byte arrays. It introduces more efficient implementations using direct memory operations instead of byte-by-byte assembly, along with support for pinned byte arrays and buffer-based I/O operations.
Key changes include:
- Added little-endian read/write functions for 16, 24, 32, 40, and 64-bit values
- Introduced
PinnedByteArraytype and related operations - Implemented buffer-based socket send/receive functions
- Enhanced I/O operations to work with pinned buffers
Reviewed Changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| unison-src/transcripts/idempotent/*.md | Updated builtin counts to reflect new functions |
| unison-src/transcripts-using-base/net.md | Added buffer-based networking examples |
| unison-runtime/src/Unison/Runtime/Foreign/Function/Type.hs | Added new foreign function types for little-endian operations |
| unison-runtime/src/Unison/Runtime/Foreign/Function.hs | Implemented little-endian read/write operations using GHC primitives |
| unison-runtime/src/Unison/Runtime/Builtin.hs | Declared new foreign functions |
| unison-core/src/Unison/Type.hs | Added PinnedByteArray type reference |
| parser-typechecker/src/Unison/KindInference/Generate.hs | Updated constraint tree for new type |
| parser-typechecker/src/Unison/Builtin.hs | Added type signatures for new operations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
…rorama/littleEndianByteArray
| withMutableByteArrayContents :: (PA.PrimBase m) => PA.MutableByteArray (PA.PrimState m) -> (Ptr Word8 -> m a) -> m a | ||
| {-# INLINE withMutableByteArrayContents #-} | ||
| withMutableByteArrayContents mba = | ||
| PA.keepAlive (PA.mutableByteArrayContents mba) |
There was a problem hiding this comment.
So, I don't understand what's going on here. It doesn't look like it should even type check. Is Github showing me the wrong diff or something?
dolio
left a comment
There was a problem hiding this comment.
Other than my confusion mentioned above, this looks good. The thing with the native endianness is nice; I didn't know that information was exposed by GHC.
Looks well-typed to me given the type of keepAlive in Prim 8.0.0 |
|
Oh, that explains it. They changed the type of I guess I have one more concern about this, though. You are calling So, I think you might need to be calling I guess there's also caveats there about the boxed |
Overview
This adds builtin little-endian equivalents for all the big-endian read operations on mutable and immutable byte arrays.
Implementation notes
I changed the way these reads are implemented. Previously, e.g. the 64-bit read would be eight one-byte reads, assembled into a
Natwith shifts. The new implementation is a single 64-bit read. It takes into account the native endianness of the machine we're running on and re-orders the bytes if necessary, using a single additional primop.Interesting/controversial decisions
The new implementation of the read/index functions use some GHC internals, so this code is very much not portable.
Test coverage
Every combination of reads and writes is tested in
builtins.md.Dependencies
Depends on #5832 but probably doesn't have to.