Skip to content

feat: build and run on Windows#8

Merged
tamirms merged 2 commits into
mainfrom
windows-build-support
Jun 22, 2026
Merged

feat: build and run on Windows#8
tamirms merged 2 commits into
mainfrom
windows-build-support

Conversation

@tamirms

@tamirms tamirms commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

The core read/write paths used unix.Pwrite/Pread on raw fds, so the library failed to compile on Windows — breaking downstream importers there.

This swaps them for portable *os.File.WriteAt/ReadAt and routes the !linux && !darwin prealloc fallback through os.File.Truncate (no unix import). The unsorted builder's anonymous temp files use unlink-while-open on Unix and FILE_FLAG_DELETE_ON_CLOSE on Windows, so the whole library builds and runs on Windows.

Path Unix Windows
Query/read + sorted builder
Unsorted builder ✅ (FILE_FLAG_DELETE_ON_CLOSE)

On Unix WriteAt is the same pwrite syscall and keeps the lock-free concurrent-write property. Build throughput unchanged within ~5% noise (10M keys, ptrhash).

Verified: go test -race ./..., gofmt/vet/golangci-lint, and GOOS=windows/freebsd cross-compile.

Not verified / known limitations (CI is ubuntu+macos only, so the Windows runtime path is unexercised):

  • Windows temp-dir cleanup is best-effort. The DELETE_ON_CLOSE files are reclaimed on close (and on crash), but the temp directory is removed in cleanup(), so an abnormal exit leaks an empty streamhash-parts-* dir. A handle held by AV/indexer at close time can also leave the dir briefly in delete-pending and skip removal.
  • A Windows CI runner is the recommended follow-up before relying on the Windows runtime path.

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings June 22, 2026 12:34

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes golang.org/x/sys/unix usage from core read/write paths and replaces raw-fd pread/pwrite syscalls with portable *os.File.ReadAt/WriteAt, enabling the library to compile (and the sorted builder + query/read paths to run) on Windows while keeping platform-specific preallocation behind OS build tags.

Changes:

  • Replaced unix.Pwrite/Pread call sites with (*os.File).WriteAt/ReadAt and adjusted APIs to pass *os.File / io.ReaderAt instead of raw fds.
  • Refactored preallocFile to accept *os.File, using ftruncate-free fallback (os.File.Truncate) on !linux && !darwin and retaining fallocate/F_PREALLOCATE on Linux/macOS.
  • Updated platform documentation to reflect Windows compilation support and the unsorted builder limitation.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
platform_other.go Removes unix dependency by using os.File.Truncate for the non-Linux/Darwin prealloc fallback (covers Windows).
platform_linux.go Updates preallocFile to accept *os.File and uses Fallocate(int(f.Fd()), ...).
platform_darwin.go Updates preallocFile to accept *os.File and uses FcntlFstore(f.Fd(), ...).
index_writer.go Switches positioned writes from unix.Pwrite on cached fds to file.WriteAt.
builder_unsorted.go Switches unsorted writer flush from unix.Pwrite to WriteAt and removes cached fd state.
builder_unsorted_finish.go Switches partition reads from unix.Pread to io.ReaderAt.ReadAt.
CLAUDE.md Updates platform support notes to reflect Windows compilation and unsorted builder limitation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread builder_unsorted.go
Comment thread builder_unsorted.go Outdated
@tamirms tamirms force-pushed the windows-build-support branch from 7afc6c5 to 7c9a82c Compare June 22, 2026 12:43
@tamirms tamirms changed the title feat: build on Windows via portable *os.File positional I/O feat: build on Windows Jun 22, 2026
@tamirms tamirms force-pushed the windows-build-support branch from 7c9a82c to 2422b5b Compare June 22, 2026 13:24
@tamirms tamirms changed the title feat: build on Windows feat: build and run on Windows Jun 22, 2026
@tamirms tamirms requested a review from Copilot June 22, 2026 13:29

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Comment thread builder_unsorted.go
Comment thread platform_windows.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Comment thread builder_unsorted_finish.go Outdated
Comment thread platform_windows.go
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>
@tamirms tamirms force-pushed the windows-build-support branch from 89b83c7 to 0037986 Compare June 22, 2026 14:40
@tamirms tamirms requested a review from a team June 22, 2026 14:43
@chowbao

chowbao commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

nit: It doesn't look like the ci/cd doesn't actually check windows. I only see macos/ubuntu

@tamirms

tamirms commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

nit: It doesn't look like the ci/cd doesn't actually check windows. I only see macos/ubuntu

yes, this is a follow up task which has been de-prioritized but can be done later

@tamirms tamirms enabled auto-merge (squash) June 22, 2026 15:00
@tamirms tamirms merged commit 9e08e05 into main Jun 22, 2026
11 checks passed
@tamirms tamirms deleted the windows-build-support branch June 22, 2026 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants