Skip to content

Complete merges on writer release and add a long-lived collection writer - #14

Merged
iamcarbon merged 1 commit into
mainfrom
claude/per-document-helper-perf-b80fb5
Sep 19, 2026
Merged

iamcarbon merged 1 commit into
mainfrom
claude/per-document-helper-perf-b80fb5

Conversation

@myluma

@myluma myluma commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

Follow-up to the review note that the per-document helpers (index.add(doc), upsert, delete(matching:)) are expensive in a loop. Measuring it turned up a second, worse problem in the same path, fixed here alongside the API and doc changes the note asked for.

What changed

  • Merges now complete when a writer is released (rust/src/lib.rs). tantivy kills its merge threads when an IndexWriter is dropped, and a commit schedules merges asynchronously, so every scoped helper (writer → commit → release) abandoned its merges and left one segment per call behind. tantivy_writer_free now calls tantivy's public wait_merging_threads() instead of dropping. tantivy itself is unchanged; this is shim-only.
  • SearchCollection.writer(heapSize:) returns a new typed SearchCollection.Writer with add, add(contentsOf:), upsert, remove(idField:id:), remove(matching:), removeAll, commit (commits + reloads), rollback, and an indexWriter escape hatch. write {} is unchanged to avoid a source break.
  • Docs. Every one-shot writing helper on Index and SearchCollection now says it opens a writer, commits and reloads per call and points at add(contentsOf:), write {} or a long-lived writer. IndexWriter type docs and the README explain the cost and the wait-on-release behaviour.
  • Tests. 21 new tests: typed-writer ordering, lock contention with the helpers, encoding failure, lifecycle, .onCommit policy, persistence; and regression tests that scoped adds no longer leave one segment per call, in memory and on disk. 414 tests pass.

Measurements (200 small docs, release build, this machine)

Path In memory On disk Segments left
index.add(doc) in a loop, before 318 ms 20.4 s 200
index.add(contentsOf:) once 1 ms 0.14 s 3
long-lived writer, commit per doc 128 ms 13.0 s 4
index.add(doc) in a loop, after 293 ms 12.9 s 4

Writer creation itself is cheap (~0.1 ms). The per-call cost is the commit (segment write + fsync, ~65 ms on disk here); the segment growth was the hidden cost, making searches on the loop-built index ~10x slower.

Reviewer notes

  • Ships with the next release. The merge wait is in the FFI shim, so consumers of the published xcframework only get it once release.sh rebuilds the binary. Until then the docs describe behaviour the current binary lacks.
  • Deinit can block. Releasing a writer during a large in-flight merge now waits for it (milliseconds for batch-sized segments). Documented on IndexWriter; avoid releasing a busy writer on the main thread.
  • A merge error during the wait is swallowed in tantivy_writer_free (deinit cannot report it; committed data is unaffected and the next commit retries).

🤖 Generated with Claude Code

tantivy kills its merge threads when an IndexWriter is dropped, and a commit
schedules merges asynchronously, so every scoped helper (write/add/upsert/
delete: writer, commit, release) abandoned its merges and left one segment
per call behind. 200 per-document adds produced a 200-segment index whose
searches were ~10x slower than the batch-built equivalent. tantivy_writer_free
now calls wait_merging_threads() instead of dropping the writer; the same loop
leaves 4 segments and, on disk, runs faster because the index stays small.

SearchCollection gains writer(heapSize:) returning a typed Writer (add,
add(contentsOf:), upsert, remove, removeAll, commit, rollback, indexWriter)
so callers whose models arrive one at a time can hold one writer and commit
per batch instead of paying a writer + commit + reload per document.

The single-document helpers on Index and SearchCollection, the IndexWriter
type docs and the README now state the per-call cost plainly and point at
the batch and long-lived alternatives.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@iamcarbon
iamcarbon merged commit 92ae362 into main Sep 19, 2026
1 check passed
myluma pushed a commit that referenced this pull request Sep 19, 2026
Resolves the conflict in tantivy_writer_free: keep main's wait for
background merges (#14) and run it inside catch_unwind, so a panic in
the drop path cannot escape the extern "C" boundary and abort.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.

2 participants