Skip to content

Commit 9e08e05

Browse files
tamirmsclaude
andauthored
feat: build and run on Windows via portable file I/O (#8)
The core read/write paths used unix.Pwrite/Pread on raw fds, and platform_other.go imported x/sys/unix, so the library failed to compile on Windows — breaking downstream importers there. - Replace raw-fd syscalls with *os.File.WriteAt/ReadAt (pwrite/pread on Unix, OVERLAPPED on Windows; same lock-free concurrent-write property). - Route the !linux && !darwin prealloc fallback through os.File.Truncate (no unix import); only platform_linux/darwin keep x/sys/unix. - The unsorted builder's anonymous temp files use unlink-while-open on Unix (platform_unix.go) and FILE_FLAG_DELETE_ON_CLOSE on Windows (platform_windows.go); the temp dir is removed in cleanup. Build throughput unchanged within ~5% noise. Windows is compile-verified via cross-compile but not exercised in CI (ubuntu+macos only); the temp directory is best-effort cleaned and may leak on a Windows crash. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent c86fc06 commit 9e08e05

10 files changed

Lines changed: 103 additions & 67 deletions

CLAUDE.md

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,18 +25,21 @@ 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`/`platform_darwin.go` use `golang.org/x/sys/unix` (for
29+
`fallocate`/`fcntl`); `platform_other.go` (`!linux && !darwin`, incl. Windows)
30+
uses `os.File.Truncate`. File I/O is portable `*os.File.WriteAt`/`ReadAt`, and
31+
the unsorted builder's temp files use unlink-while-open on Unix and
32+
`FILE_FLAG_DELETE_ON_CLOSE` on Windows. Builds and runs on Windows, but CI is
33+
ubuntu + macos only — so Windows is compile-verified, not runtime-tested.
3134

3235
## Concurrency
3336

3437
The builders are heavily concurrent and are the main correctness risk:
3538
- **Sorted parallel** (`builder_parallel.go`): an `errgroup` worker pool builds
3639
blocks; a single writer goroutine emits them in block order via channels.
3740
- **Unsorted** (`builder_unsorted*.go`): concurrent writers each own a temp file
38-
(lock-free `pwrite`); the finish phase uses a reader goroutine pool with
39-
per-slot fences.
41+
(lock-free positional `WriteAt`); the finish phase uses a reader goroutine pool
42+
with per-slot fences.
4043

4144
Treat the race detector as a first-class gate, and keep per-test timeouts on the
4245
channel/`errgroup` code so a deadlock fails the run instead of hanging it. If a

builder_unsorted.go

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,11 @@ import (
88
"math"
99
"math/bits"
1010
"os"
11+
"path/filepath"
1112
"sync"
1213
"sync/atomic"
1314
"unsafe"
1415

15-
"golang.org/x/sys/unix"
16-
1716
intbits "github.com/stellar/streamhash/internal/bits"
1817
"github.com/stellar/streamhash/internal/sherr"
1918
)
@@ -124,7 +123,7 @@ type unsortedBuffer struct {
124123
numWriters int // total writer count (from config)
125124
regionSize int64 // bytes per partition region per writer file
126125
writerFiles []*os.File // [numWriters]
127-
writerFds []int // [numWriters] fd cache for pwrite/pread
126+
partsDir string // temp dir; removed in cleanup
128127
writerCursors [][]int64 // [numWriters][numPartitions] byte offset within region
129128
nextWriterID int // next writerID to assign
130129
writerMu sync.Mutex // protects nextWriterID
@@ -206,7 +205,6 @@ func newUnsortedBuffer(cfg *buildConfig, numBlocks uint32, numWriters int) (*uns
206205
numWriters: numWriters,
207206
regionSize: regionSize,
208207
writerFiles: make([]*os.File, numWriters),
209-
writerFds: make([]int, numWriters),
210208
writerCursors: make([][]int64, numWriters),
211209
}
212210
for w := range numWriters {
@@ -220,8 +218,8 @@ func newUnsortedBuffer(cfg *buildConfig, numBlocks uint32, numWriters int) (*uns
220218
return u, nil
221219
}
222220

223-
// createWriterFiles creates the temp directory and per-writer files.
224-
// Each writer gets a single file pre-allocated via fallocate.
221+
// createWriterFiles creates the temp directory and per-writer files, each
222+
// pre-allocated then detached (unlinked on Unix, DELETE_ON_CLOSE on Windows).
225223
func (u *unsortedBuffer) createWriterFiles(tempDir string) error {
226224
if tempDir == "" {
227225
tempDir = os.TempDir()
@@ -230,33 +228,34 @@ func (u *unsortedBuffer) createWriterFiles(tempDir string) error {
230228
if err != nil {
231229
return err
232230
}
231+
u.partsDir = dir
233232

234233
fileSize := u.regionSize * int64(u.numPartitions)
235234
for w := range u.numWriters {
236-
path := fmt.Sprintf("%s/writer-%03d", dir, w)
237-
f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE, 0600)
235+
path := filepath.Join(dir, fmt.Sprintf("writer-%03d", w))
236+
f, err := openWriterFile(path)
238237
if err != nil {
239238
u.closeWriterFiles()
240239
os.RemoveAll(dir)
241240
return fmt.Errorf("create writer %d: %w", w, err)
242241
}
243-
if err := preallocFile(int(f.Fd()), fileSize); err != nil {
242+
if err := preallocFile(f, fileSize); err != nil {
244243
f.Close()
245244
u.closeWriterFiles()
246245
os.RemoveAll(dir)
247-
return fmt.Errorf("fallocate writer %d (%d bytes): %w", w, fileSize, err)
246+
return fmt.Errorf("preallocate writer %d (%d bytes): %w", w, fileSize, err)
248247
}
249-
if err := os.Remove(path); err != nil {
248+
if err := unlinkWhileOpen(path); err != nil {
250249
f.Close()
251250
u.closeWriterFiles()
252251
os.RemoveAll(dir)
253252
return fmt.Errorf("unlink writer %d: %w", w, err)
254253
}
255254
u.writerFiles[w] = f
256-
u.writerFds[w] = int(f.Fd())
257255
}
258256

259-
// All files are unlinked — remove the now-empty temp directory.
257+
// Unix: files are unlinked, so the dir is empty and removed now. Windows:
258+
// DELETE_ON_CLOSE files persist until close, so cleanup() removes the dir.
260259
os.Remove(dir)
261260
return nil
262261
}
@@ -399,9 +398,9 @@ func (ws *writerState) doFlush(bufIdx int) error {
399398
return fmt.Errorf("partition %d overflow in writer %d: region capacity exceeded", p, ws.writerID)
400399
}
401400

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

407406
u.writerCursors[ws.writerID][p] = cursor + int64(needed)
@@ -429,10 +428,8 @@ func (ws *writerState) flushAll() error {
429428
}
430429

431430
// 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.
431+
// Abort paths (Close/cleanupAll) must drain before closing the file, or a
432+
// background WriteAt aborts with ErrClosed and drops its data.
436433
func (ws *writerState) drainFlush() {
437434
ws.flushWg.Wait()
438435
}

builder_unsorted_finish.go

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@ import (
55
"encoding/binary"
66
"errors"
77
"fmt"
8+
"io"
89
"math/bits"
10+
"os"
911
"sync"
1012
"unsafe"
1113

12-
"golang.org/x/sys/unix"
13-
1414
intbits "github.com/stellar/streamhash/internal/bits"
1515
"github.com/stellar/streamhash/internal/sherr"
1616
)
@@ -93,7 +93,7 @@ func (u *unsortedBuffer) readPartition(p int, slot *readSlot) ([][]routedEntry,
9393
if regionBytes == 0 {
9494
continue
9595
}
96-
if err := u.readRegion(u.writerFds[w], regionStart, regionBytes, readBuf, entrySize,
96+
if err := u.readRegion(u.writerFiles[w], regionStart, regionBytes, readBuf, entrySize,
9797
func(data []byte) {
9898
k0 := binary.LittleEndian.Uint64(data[0:8])
9999
k1 := binary.LittleEndian.Uint64(data[8:16])
@@ -113,21 +113,18 @@ func (u *unsortedBuffer) readPartition(p int, slot *readSlot) ([][]routedEntry,
113113
return blockEntries, nil
114114
}
115115

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 {
116+
// readRegion reads regionBytes from r at offset and invokes fn for each
117+
// fixed-size entry. ReadAt is positional, so readers can share one source.
118+
func (u *unsortedBuffer) readRegion(r io.ReaderAt, offset, regionBytes int64, readBuf []byte, entrySize int, fn func([]byte)) error {
119119
pos := offset
120120
remaining := regionBytes
121121
leftoverN := 0
122122

123123
for remaining > 0 {
124124
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)
125+
nr, err := r.ReadAt(readBuf[leftoverN:leftoverN+int(toRead)], pos)
126+
if int64(nr) != toRead {
127+
return fmt.Errorf("readRegion: short read at offset %d: got %d of %d bytes (%d remaining): %w", pos, nr, toRead, remaining, err)
131128
}
132129
pos += int64(nr)
133130
remaining -= int64(nr)
@@ -143,10 +140,6 @@ func (u *unsortedBuffer) readRegion(fd int, offset, regionBytes int64, readBuf [
143140
if len(data) > 0 {
144141
leftoverN = copy(readBuf, data)
145142
}
146-
147-
if err != nil {
148-
return err
149-
}
150143
}
151144
return nil
152145
}
@@ -225,8 +218,9 @@ func (u *unsortedBuffer) prepareForRead(numSlots int) error {
225218
return nil
226219
}
227220

228-
// cleanup closes all writer fds. The temp directory is removed during
229-
// createWriterFiles after all files are unlinked. Idempotent.
221+
// cleanup closes writer files and removes the temp dir. Best-effort and
222+
// idempotent: teardown errors are ignored so they can't fail a built index
223+
// (cleanup runs before finalize on the success path).
230224
func (u *unsortedBuffer) cleanup() error {
231225
for w, f := range u.writerFiles {
232226
if f != nil {
@@ -235,6 +229,10 @@ func (u *unsortedBuffer) cleanup() error {
235229
}
236230
}
237231
u.writerFiles = nil
232+
if u.partsDir != "" {
233+
os.RemoveAll(u.partsDir)
234+
u.partsDir = ""
235+
}
238236
return nil
239237
}
240238

builder_unsorted_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"math/bits"
66
randv2 "math/rand/v2"
77
"os"
8+
"runtime"
89
"sync"
910
"sync/atomic"
1011
"testing"
@@ -132,6 +133,9 @@ func TestUnsortedBuffer_CleanupIdempotent(t *testing.T) {
132133
}
133134

134135
func TestUnsortedBuffer_TempDirRemovedAfterCreate(t *testing.T) {
136+
if runtime.GOOS == "windows" {
137+
t.Skip("Windows DELETE_ON_CLOSE temp files persist until cleanup, so the dir is not empty right after create")
138+
}
135139
tmpDir := t.TempDir()
136140
cfg := &buildConfig{totalKeys: 100, payloadSize: 4, unsortedTempDir: tmpDir}
137141
numBlocks, _ := numBlocksForAlgo(AlgoBijection, 100, 4, 0)

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
}

0 commit comments

Comments
 (0)