Skip to content

Commit 7c9a82c

Browse files
tamirmsclaude
andcommitted
feat: build on Windows via portable *os.File positional I/O
The core write/read paths imported golang.org/x/sys/unix (unix.Pwrite/Pread on cached raw int fds), as did platform_other.go, so the library failed to compile on Windows — breaking any downstream module that imports it on a Windows runner. Replace the raw-fd syscalls with *os.File.WriteAt/ReadAt, which map to pwrite/pread on Unix and OVERLAPPED I/O on Windows and preserve the lock-free concurrent positioned-write property (Go's internal/poll uses incref, not a write lock, for positional I/O). Change preallocFile to take *os.File and route the !linux && !darwin fallback through os.File.Truncate, which needs no unix import and also covers Windows, so no dedicated platform_windows.go is required. Only platform_linux.go (fallocate) and platform_darwin.go (F_PREALLOCATE) still use x/sys/unix. Result: the sorted builder and all query/read paths build and work on Windows. The unsorted builder still relies on the POSIX unlink-while-open idiom for its temp files, which Windows rejects, so NewUnsortedBuilder is unsupported there. Benchmarks (10M keys, ptrhash, 7 iters, before/after) show build throughput unchanged within noise: sorted +1.8%, unsorted +1.3%, unsorted cw=4 +3.0% medians — all within the ~5% band. The added WriteAt/ReadAt cost is a couple of atomic refcount ops per syscall, dwarfed by the ~µs syscall. Windows compilation verified via GOOS=windows cross-compile; CI still runs ubuntu + macos only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 4db19d2 commit 7c9a82c

7 files changed

Lines changed: 52 additions & 56 deletions

CLAUDE.md

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,13 @@ Three build paths: `NewSortedBuilder` (sorted input), `NewUnsortedBuilder`
2525

2626
## Platform
2727

28-
Unix-only: `platform_{linux,darwin,other}.go` all import `golang.org/x/sys/unix`
29-
(for `fallocate`/`fcntl`), so the library does **not** build on Windows. CI runs
30-
on ubuntu + macos only.
28+
`platform_linux.go` and `platform_darwin.go` use `golang.org/x/sys/unix` (for
29+
`fallocate`/`fcntl`); the `platform_other.go` fallback (`!linux && !darwin`,
30+
which also covers Windows) uses only `os.File.Truncate`. The library compiles
31+
for Windows (cross-compile verified; **not** yet exercised in CI), and the
32+
sorted builder plus all query/read paths work there. The unsorted builder
33+
(`NewUnsortedBuilder`) relies on the POSIX unlink-while-open idiom, which Windows
34+
rejects, so it is unsupported there. CI runs on ubuntu + macos only.
3135

3236
## Concurrency
3337

builder_unsorted.go

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ import (
1212
"sync/atomic"
1313
"unsafe"
1414

15-
"golang.org/x/sys/unix"
16-
1715
intbits "github.com/stellar/streamhash/internal/bits"
1816
"github.com/stellar/streamhash/internal/sherr"
1917
)
@@ -124,7 +122,6 @@ type unsortedBuffer struct {
124122
numWriters int // total writer count (from config)
125123
regionSize int64 // bytes per partition region per writer file
126124
writerFiles []*os.File // [numWriters]
127-
writerFds []int // [numWriters] fd cache for pwrite/pread
128125
writerCursors [][]int64 // [numWriters][numPartitions] byte offset within region
129126
nextWriterID int // next writerID to assign
130127
writerMu sync.Mutex // protects nextWriterID
@@ -206,7 +203,6 @@ func newUnsortedBuffer(cfg *buildConfig, numBlocks uint32, numWriters int) (*uns
206203
numWriters: numWriters,
207204
regionSize: regionSize,
208205
writerFiles: make([]*os.File, numWriters),
209-
writerFds: make([]int, numWriters),
210206
writerCursors: make([][]int64, numWriters),
211207
}
212208
for w := range numWriters {
@@ -240,20 +236,21 @@ func (u *unsortedBuffer) createWriterFiles(tempDir string) error {
240236
os.RemoveAll(dir)
241237
return fmt.Errorf("create writer %d: %w", w, err)
242238
}
243-
if err := preallocFile(int(f.Fd()), fileSize); err != nil {
239+
if err := preallocFile(f, fileSize); err != nil {
244240
f.Close()
245241
u.closeWriterFiles()
246242
os.RemoveAll(dir)
247243
return fmt.Errorf("fallocate writer %d (%d bytes): %w", w, fileSize, err)
248244
}
245+
// Unlink while open (POSIX anonymous-file idiom): the handle keeps the
246+
// data reachable. Windows rejects this, so unsorted is unsupported there.
249247
if err := os.Remove(path); err != nil {
250248
f.Close()
251249
u.closeWriterFiles()
252250
os.RemoveAll(dir)
253251
return fmt.Errorf("unlink writer %d: %w", w, err)
254252
}
255253
u.writerFiles[w] = f
256-
u.writerFds[w] = int(f.Fd())
257254
}
258255

259256
// All files are unlinked — remove the now-empty temp directory.
@@ -399,9 +396,9 @@ func (ws *writerState) doFlush(bufIdx int) error {
399396
return fmt.Errorf("partition %d overflow in writer %d: region capacity exceeded", p, ws.writerID)
400397
}
401398

402-
_, err := unix.Pwrite(u.writerFds[ws.writerID], buf[:needed], off)
399+
_, err := u.writerFiles[ws.writerID].WriteAt(buf[:needed], off)
403400
if err != nil {
404-
return fmt.Errorf("pwrite partition %d writer %d: %w", p, ws.writerID, err)
401+
return fmt.Errorf("write partition %d writer %d: %w", p, ws.writerID, err)
405402
}
406403

407404
u.writerCursors[ws.writerID][p] = cursor + int64(needed)
@@ -429,10 +426,8 @@ func (ws *writerState) flushAll() error {
429426
}
430427

431428
// drainFlush waits for any in-flight background flush goroutine to finish.
432-
// Abort paths (Close/cleanupAll) must call this before closing the writer's fd:
433-
// doFlush issues unix.Pwrite on the cached fd from a background goroutine, so
434-
// closing the fd while that goroutine is mid-write races a closed (and possibly
435-
// recycled) descriptor. The normal Finish path drains via flushAll instead.
429+
// Abort paths (Close/cleanupAll) must drain before closing the file, or a
430+
// background WriteAt aborts with ErrClosed and drops its data.
436431
func (ws *writerState) drainFlush() {
437432
ws.flushWg.Wait()
438433
}

builder_unsorted_finish.go

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,11 @@ import (
55
"encoding/binary"
66
"errors"
77
"fmt"
8+
"io"
89
"math/bits"
910
"sync"
1011
"unsafe"
1112

12-
"golang.org/x/sys/unix"
13-
1413
intbits "github.com/stellar/streamhash/internal/bits"
1514
"github.com/stellar/streamhash/internal/sherr"
1615
)
@@ -93,7 +92,7 @@ func (u *unsortedBuffer) readPartition(p int, slot *readSlot) ([][]routedEntry,
9392
if regionBytes == 0 {
9493
continue
9594
}
96-
if err := u.readRegion(u.writerFds[w], regionStart, regionBytes, readBuf, entrySize,
95+
if err := u.readRegion(u.writerFiles[w], regionStart, regionBytes, readBuf, entrySize,
9796
func(data []byte) {
9897
k0 := binary.LittleEndian.Uint64(data[0:8])
9998
k1 := binary.LittleEndian.Uint64(data[8:16])
@@ -113,21 +112,18 @@ func (u *unsortedBuffer) readPartition(p int, slot *readSlot) ([][]routedEntry,
113112
return blockEntries, nil
114113
}
115114

116-
// readRegion reads regionBytes from fd at offset using pread, calling fn for
117-
// each entry. Uses pread (not seek+read) for concurrent reader safety.
118-
func (u *unsortedBuffer) readRegion(fd int, offset, regionBytes int64, readBuf []byte, entrySize int, fn func([]byte)) error {
115+
// readRegion reads regionBytes from r at offset and invokes fn for each
116+
// fixed-size entry. ReadAt is positional, so readers can share one source.
117+
func (u *unsortedBuffer) readRegion(r io.ReaderAt, offset, regionBytes int64, readBuf []byte, entrySize int, fn func([]byte)) error {
119118
pos := offset
120119
remaining := regionBytes
121120
leftoverN := 0
122121

123122
for remaining > 0 {
124123
toRead := min(int64(len(readBuf)-leftoverN), remaining)
125-
nr, err := unix.Pread(fd, readBuf[leftoverN:leftoverN+int(toRead)], pos)
126-
if nr == 0 {
127-
if err != nil {
128-
return err
129-
}
130-
return fmt.Errorf("pread: unexpected zero read at offset %d with %d remaining", pos, remaining)
124+
nr, err := r.ReadAt(readBuf[leftoverN:leftoverN+int(toRead)], pos)
125+
if int64(nr) != toRead {
126+
return fmt.Errorf("readRegion: short read at offset %d: got %d of %d bytes (%d remaining): %w", pos, nr, toRead, remaining, err)
131127
}
132128
pos += int64(nr)
133129
remaining -= int64(nr)
@@ -143,10 +139,6 @@ func (u *unsortedBuffer) readRegion(fd int, offset, regionBytes int64, readBuf [
143139
if len(data) > 0 {
144140
leftoverN = copy(readBuf, data)
145141
}
146-
147-
if err != nil {
148-
return err
149-
}
150142
}
151143
return nil
152144
}

index_writer.go

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,12 @@ import (
88
"os"
99

1010
"github.com/cespare/xxhash/v2"
11-
"golang.org/x/sys/unix"
1211
)
1312

14-
// indexWriter handles writing index data to disk using pwrite-based writes.
13+
// indexWriter writes index data to disk via positional *os.File.WriteAt calls.
1514
// File layout: [Header 64B][UserMetaLen 4B][UserMeta][AlgoConfigLen 4B][AlgoConfig][RAM Index (N+1)×10B][Payload Region][Metadata Region][Footer 32B]
1615
type indexWriter struct {
1716
file *os.File
18-
fd int // cached file descriptor for pwrite syscalls
1917

2018
// Region offsets (computed upfront for separated layout)
2119
payloadRegionOffset uint64
@@ -84,14 +82,13 @@ func newIndexWriter(path string, cfg *buildConfig, numBlocks uint32, algo blockB
8482
metadataRegionOffset := payloadRegionOffset + payloadRegionSize
8583

8684
// Pre-allocate disk blocks for space reservation
87-
if err := preallocFile(int(file.Fd()), int64(estimatedSize)); err != nil {
85+
if err := preallocFile(file, int64(estimatedSize)); err != nil {
8886
primaryErr := fmt.Errorf("failed to allocate disk space: %w", err)
8987
return nil, errors.Join(primaryErr, file.Close())
9088
}
9189

9290
iw := &indexWriter{
9391
file: file,
94-
fd: int(file.Fd()),
9592
payloadRegionOffset: payloadRegionOffset,
9693
metadataRegionOffset: metadataRegionOffset,
9794
userMetadataOffset: userMetadataOffset,
@@ -136,7 +133,7 @@ func (iw *indexWriter) writePayloadsDirect(data []byte, offset uint64) error {
136133
if absoluteOffset+uint64(len(data)) > iw.metadataRegionOffset {
137134
return fmt.Errorf("writePayloadsDirect: write exceeds payload region boundary")
138135
}
139-
_, err := unix.Pwrite(iw.fd, data, int64(absoluteOffset))
136+
_, err := iw.file.WriteAt(data, int64(absoluteOffset))
140137
return err
141138
}
142139

@@ -165,7 +162,7 @@ func (iw *indexWriter) recordAndWriteMetadata(metadata []byte) error {
165162
})
166163

167164
if len(metadata) > 0 {
168-
if _, err := unix.Pwrite(iw.fd, metadata, int64(iw.metadataWriteOffset)); err != nil {
165+
if _, err := iw.file.WriteAt(metadata, int64(iw.metadataWriteOffset)); err != nil {
169166
return err
170167
}
171168
if _, err := iw.metadataHasher.Write(metadata); err != nil {
@@ -202,7 +199,7 @@ func (iw *indexWriter) commitBlockWithData(metadata []byte, payloads []byte, num
202199
entrySize := iw.cfg.payloadSize + iw.cfg.fingerprintSize
203200
if entrySize > 0 && numKeys > 0 {
204201
payloadOffset := iw.payloadRegionOffset + iw.keyCount*uint64(entrySize)
205-
if _, err := unix.Pwrite(iw.fd, payloads, int64(payloadOffset)); err != nil {
202+
if _, err := iw.file.WriteAt(payloads, int64(payloadOffset)); err != nil {
206203
return err
207204
}
208205
iw.foldPayloadHash(xxhash.Sum64(payloads))
@@ -245,7 +242,7 @@ func (iw *indexWriter) finalize() error {
245242
// Write header at offset 0
246243
var headerBuf [headerSize]byte
247244
iw.header.encodeTo(headerBuf[:])
248-
if _, err := unix.Pwrite(iw.fd, headerBuf[:], 0); err != nil {
245+
if _, err := iw.file.WriteAt(headerBuf[:], 0); err != nil {
249246
primaryErr := fmt.Errorf("write header failed: %w", err)
250247
return errors.Join(primaryErr, iw.close())
251248
}
@@ -255,7 +252,7 @@ func (iw *indexWriter) finalize() error {
255252
userMetaBuf := make([]byte, 4+len(iw.userMetadata))
256253
binary.LittleEndian.PutUint32(userMetaBuf, uint32(len(iw.userMetadata)))
257254
copy(userMetaBuf[4:], iw.userMetadata)
258-
if _, err := unix.Pwrite(iw.fd, userMetaBuf, int64(iw.userMetadataOffset)); err != nil {
255+
if _, err := iw.file.WriteAt(userMetaBuf, int64(iw.userMetadataOffset)); err != nil {
259256
primaryErr := fmt.Errorf("write user metadata failed: %w", err)
260257
return errors.Join(primaryErr, iw.close())
261258
}
@@ -267,7 +264,7 @@ func (iw *indexWriter) finalize() error {
267264
if algoConfigLen > 0 {
268265
iw.algo.EncodeGlobalConfigInto(algoConfigBuf[4:])
269266
}
270-
if _, err := unix.Pwrite(iw.fd, algoConfigBuf, int64(iw.algoConfigOffset)); err != nil {
267+
if _, err := iw.file.WriteAt(algoConfigBuf, int64(iw.algoConfigOffset)); err != nil {
271268
primaryErr := fmt.Errorf("write algo config failed: %w", err)
272269
return errors.Join(primaryErr, iw.close())
273270
}
@@ -278,7 +275,7 @@ func (iw *indexWriter) finalize() error {
278275
offset := i * int(ramIndexEntrySize)
279276
encodeRAMIndexEntryTo(entry, ramIndexBuf[offset:])
280277
}
281-
if _, err := unix.Pwrite(iw.fd, ramIndexBuf, int64(iw.ramIndexOffset)); err != nil {
278+
if _, err := iw.file.WriteAt(ramIndexBuf, int64(iw.ramIndexOffset)); err != nil {
282279
primaryErr := fmt.Errorf("write RAM index failed: %w", err)
283280
return errors.Join(primaryErr, iw.close())
284281
}
@@ -290,7 +287,7 @@ func (iw *indexWriter) finalize() error {
290287
}
291288
var footerBuf [footerSize]byte
292289
ftr.encodeTo(footerBuf[:])
293-
if _, err := unix.Pwrite(iw.fd, footerBuf[:], int64(iw.metadataWriteOffset)); err != nil {
290+
if _, err := iw.file.WriteAt(footerBuf[:], int64(iw.metadataWriteOffset)); err != nil {
294291
primaryErr := fmt.Errorf("write footer failed: %w", err)
295292
return errors.Join(primaryErr, iw.close())
296293
}

platform_darwin.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@
22

33
package streamhash
44

5-
import "golang.org/x/sys/unix"
5+
import (
6+
"os"
7+
8+
"golang.org/x/sys/unix"
9+
)
610

711
// preallocFile pre-allocates disk blocks without changing the file size.
812
// On macOS, uses fcntl F_PREALLOCATE to reserve contiguous blocks.
@@ -11,12 +15,12 @@ import "golang.org/x/sys/unix"
1115
// on sparse pre-allocated files. ftruncate would be slightly faster (~1.7μs)
1216
// but causes page cache pressure proportional to file size, which thrashes
1317
// under memory pressure.
14-
func preallocFile(fd int, size int64) error {
18+
func preallocFile(f *os.File, size int64) error {
1519
fst := unix.Fstore_t{
1620
Flags: unix.F_ALLOCATEALL,
1721
Posmode: unix.F_PEOFPOSMODE,
1822
Offset: 0,
1923
Length: size,
2024
}
21-
return unix.FcntlFstore(uintptr(fd), unix.F_PREALLOCATE, &fst)
25+
return unix.FcntlFstore(f.Fd(), unix.F_PREALLOCATE, &fst)
2226
}

platform_linux.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,13 @@
22

33
package streamhash
44

5-
import "golang.org/x/sys/unix"
5+
import (
6+
"os"
7+
8+
"golang.org/x/sys/unix"
9+
)
610

711
// preallocFile pre-allocates disk blocks and extends the file to the given size.
8-
func preallocFile(fd int, size int64) error {
9-
return unix.Fallocate(fd, 0, 0, size)
12+
func preallocFile(f *os.File, size int64) error {
13+
return unix.Fallocate(int(f.Fd()), 0, 0, size)
1014
}

platform_other.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@
22

33
package streamhash
44

5-
import "golang.org/x/sys/unix"
5+
import "os"
66

7-
// preallocFile extends the file to the given size.
8-
// Fallback for platforms without fallocate: uses ftruncate to set file size
9-
// so pwrite has allocated blocks to write into.
10-
func preallocFile(fd int, size int64) error {
11-
return unix.Ftruncate(fd, size)
7+
// preallocFile extends the file to the given size. Fallback for platforms
8+
// without a specialized prealloc syscall (other Unixes, Windows); unlike
9+
// fallocate, Truncate leaves the file sparse rather than reserving blocks.
10+
func preallocFile(f *os.File, size int64) error {
11+
return f.Truncate(size)
1212
}

0 commit comments

Comments
 (0)