Skip to content

Replace native turbowalk with pure TypeScript workspace package#22910

Open
halgari wants to merge 16 commits into
masterfrom
halgari/APP-405
Open

Replace native turbowalk with pure TypeScript workspace package#22910
halgari wants to merge 16 commits into
masterfrom
halgari/APP-405

Conversation

@halgari

@halgari halgari commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Replaces the external native turbowalk package (which wraps winapi-bindings' WalkDir() on Windows) with a local workspace package at packages/turbowalk
  • Windows: Uses koffi FFI to call NT kernel APIs directly — no C++ compilation needed
  • Linux: Falls back to async fs.readdir + concurrent lstat (64 workers)
  • Properly checks FILE_ATTRIBUTE_HIDDEN on Windows via NT directory info (not dot-prefix convention)
  • Supports batched progress callbacks with threshold (matching old native behavior)
  • Returns Bluebird<void> to match the type signature callers expect
  • Zero import changes — the workspace package uses the same name (turbowalk) so all 30+ import sites continue to work unchanged
  • Updated 14 package.json files from catalog: to workspace:* and removed the catalog entry
  • Removed turbowalk from flatpak/generated-sources.json and the dependency report
  • Added koffi to pnpm-workspace.yaml allowBuilds (koffi ships prebuilds for all platforms — no C++ toolchain needed)

Technical approach: Win32 API selection

Why NtQueryDirectoryFile instead of FindFirstFileW/FindNextFileW

The old C++ addon (winapi-bindings) used NtQueryDirectoryFile from ntdll.dll — the low-level NT kernel directory enumeration API. This is the same API that FindFirstFileW calls internally, but with a critical difference: it returns multiple directory entries per syscall in a caller-provided buffer.

Each entry in the buffer is a variable-length FILE_FULL_DIR_INFORMATION struct containing:

  • FileAttributes (hidden, directory, reparse point flags)
  • EndOfFile (file size)
  • LastWriteTime (modification time as FILETIME)
  • FileName (variable-length UTF-16)

This means a single NtQueryDirectoryFile call replaces what would be N separate FindNextFileW calls, and no separate lstat()/GetFileAttributesW() calls are needed — all metadata comes back inline.

We initially implemented the koffi walk using FindFirstFileW/FindNextFileW, which was 2.2-2.9x slower than the old native addon because each entry required its own FFI round-trip. Switching to NtQueryDirectoryFile eliminated that per-entry overhead.

The 1KB buffer size

The old C++ addon used a 1KB buffer (static const unsigned int BUFFER_SIZE = 1024 in walk.cpp). We initially used 64KB thinking "bigger buffer = fewer syscalls = faster." Benchmarking showed the opposite:

Buffer size Median (89k entries)
1 KB 382ms
4 KB 370ms
16 KB 440ms
64 KB 586ms
256 KB 568ms
1024 KB 2160ms

Why smaller is faster: koffi marshals the entire buffer across the FFI boundary on every NtQueryDirectoryFile call. With a 64KB buffer, it copies 64KB of data even if the kernel only filled a few hundred bytes of entries. The 1KB buffer minimizes this marshaling overhead while still fitting multiple entries per syscall (a typical entry with a short filename is ~100 bytes, so 1KB fits ~8-10 entries per call).

The sweet spot is 1-4KB. We use 1KB to match the proven C++ implementation.

Other Win32 APIs considered

  • FindFirstFileW/FindNextFileW: Higher-level Win32 API. One entry per call, each crossing the FFI boundary. 2.2-2.9x slower than native.
  • GetFileInformationByHandleEx with FileIdBothDirectoryInfo: Another bulk API, but requires opening a handle first and doesn't offer meaningful advantages over NtQueryDirectoryFile.
  • NtQueryDirectoryFile with FileFullDirectoryInformation: What we use. Same API the old C++ addon used. Returns attributes, size, and timestamps inline — no separate stat calls needed.

Benchmark (NVMe — Samsung 970 EVO 1TB, Windows 11, Node 24)

All times are median of 5-10 runs on a warm filesystem cache.

Directory Total entries Old Native (C++) koffi NtQuery Ratio
src/ 1,911 8.8ms 7.0ms 0.8x (faster)
extensions/ 3,934 22.1ms 20.4ms 0.9x (faster)
node_modules/ 89,390 341ms 383ms 1.1x

The koffi FFI path matches or beats the old C++ addon on typical directory sizes (~1k-10k entries). The small regression on the 89k-entry stress test is from residual FFI call overhead (one CreateFileW + N NtQueryDirectoryFile + one CloseHandle per directory).

Note: Benchmarks run on an NVMe SSD. The performance profile should be reliable as the bottleneck is FFI call overhead, not disk I/O.

Resolves APP-405

Test plan

  • All 1188 tests pass
  • pnpm install succeeds with workspace package resolution
  • Same IEntry / IWalkOptions / Bluebird<void> return types — API-compatible
  • Benchmarked on NVMe: matches old native performance (results above)
  • FILE_ATTRIBUTE_HIDDEN properly checked via NT directory info
  • CI green

@halgari halgari self-assigned this Apr 30, 2026
halgari added 10 commits April 29, 2026 18:07
turbowalk previously wrapped winapi-bindings' WalkDir() on Windows
and fell back to a JS implementation on Linux. Replace the external
git dependency with a local workspace package that uses Node's built-in
fs.readdir + fs.lstat for all platforms.

The new implementation supports all options used in the codebase:
recurse, skipHidden, skipLinks, skipInaccessible, and details. No
import changes needed — the workspace package shadows the old name.

Resolves APP-405
- Return Bluebird<void> instead of Promise<void> to match the type
  signature callers expect (fixes typecheck on Linux CI)
- Add concurrent lstat batching (64 workers) to improve walk
  performance — roughly 2x faster than sequential on NVME
- Add bluebird as devDependency for the workspace package
purgeLinks() returns PromiseBB<void> but the .then() chain ended with
PromiseBB.map() which returns Bluebird<void[]>. Add .then(() => undefined)
to satisfy the return type. This was masked by the old turbowalk type
declarations.
Uses koffi to call Win32 directory enumeration APIs directly, which
returns attributes, size, and timestamps in a single call per entry —
no separate lstat() needed. This also enables proper FILE_ATTRIBUTE_HIDDEN
checking instead of dot-prefix convention.

Falls back to the async fs.readdir + lstat path on Linux or if koffi
is unavailable.

Benchmark (Samsung 970 EVO NVMe, Windows 11):

| Directory    | Old Native | koffi FFI | Pure TS  |
|--------------|-----------|-----------|----------|
| src/ (1.9k)  | 8.8ms     | 19.1ms    | 62.0ms   |
| ext/ (3.9k)  | 22.1ms    | 48.9ms    | 170.8ms  |
| n_m/ (89k)   | 341ms     | 1005ms    | 3426ms   |

koffi is ~2-3x the old C++ addon vs 7-10x for pure TS — acceptable
for a zero-compile solution.
NtQueryDirectoryFile (the same NT API the old C++ addon used) returns
multiple directory entries per syscall in a packed 64KB buffer, avoiding
per-entry syscall overhead. Buffer parsing is done entirely in JS.

Benchmark (Samsung 970 EVO NVMe, Windows 11):

| Directory    | Old Native | koffi NtQuery | koffi FindFirst |
|--------------|-----------|---------------|-----------------|
| src/ (1.9k)  | 8.8ms     | 13.0ms        | 19.1ms          |
| ext/ (3.9k)  | 22.1ms    | 42.7ms        | 48.9ms          |
| n_m/ (89k)   | 341ms     | 724ms         | 1005ms          |

Now ~1.5-2.1x the old native addon, down from 2.2-2.9x with FindFirst.
Benchmarking revealed smaller buffers are faster because koffi marshals
the entire buffer across the FFI boundary each call. A 1KB buffer
minimizes that overhead while still fitting multiple entries per call.

Results now match or beat the old C++ addon:
  src/ (1.9k):  8.8ms native → 7.0ms koffi
  ext/ (3.9k): 22.1ms native → 20.4ms koffi
  n_m/ (89k):   341ms native → 383ms koffi
- Add 9 tests covering recursive walk, skipHidden, recurse:false,
  non-existent dirs, file sizes, mtime, batched callbacks, and paths
- Fix error handling: check GetLastError after CreateFileW, retry on
  sharing violations (matching C++ behavior), throw on unexpected
  errors when skipInaccessible is false
- Implement details option: call NtQueryInformationFile with
  FileAllInformation to get linkCount, id, and idStr (same as C++)
- Reuse buffer and ioStatus across directories to avoid per-dir allocs
- Fix stale comment referencing FindFirstFileW
@halgari halgari marked this pull request as ready for review April 30, 2026 04:57
@halgari halgari requested a review from a team as a code owner April 30, 2026 04:57
Comment thread src/renderer/src/extensions/move_activator/index.ts Outdated
@Aragas

Aragas commented Apr 30, 2026

Copy link
Copy Markdown
Member

I'll be a bit pedantic, but I wouldn't say it's a pure TS solution, we still have a native dependency, it's just out of our maintenance scope now.
We do need to maintain still the WinAPI calls, they are just demoted to a dynamic solution instead of a strict C++ solution.
Are we sure it's the direction we want to take?

@github-actions

github-actions Bot commented May 5, 2026

Copy link
Copy Markdown

This PR has conflicts. You need to rebase the PR before it can be merged.

@github-actions

github-actions Bot commented May 6, 2026

Copy link
Copy Markdown

This PR doesn't have conflicts anymore. It can be merged after all status checks have passed and it has been reviewed.

Comment thread packages/turbowalk/package.json
@github-actions

github-actions Bot commented May 6, 2026

Copy link
Copy Markdown

This PR has conflicts. You need to rebase the PR before it can be merged.

@github-actions

github-actions Bot commented May 6, 2026

Copy link
Copy Markdown

This PR doesn't have conflicts anymore. It can be merged after all status checks have passed and it has been reviewed.

@github-actions

github-actions Bot commented May 6, 2026

Copy link
Copy Markdown

❌ Code is not formatted

To fix this:

  1. Set up the pre-commit hook (prevents future issues):

    pnpm run prepare

    This auto-formats staged files on every commit. (You will never see this error again)

  2. Format all files manually:

    pnpm run format

Then commit the formatting changes. View logs

@github-actions

github-actions Bot commented May 7, 2026

Copy link
Copy Markdown

This PR has conflicts. You need to rebase the PR before it can be merged.

@github-actions

Copy link
Copy Markdown

This PR has been marked as stale due to inactivity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants