Compaction conversion - #711
Draft
jshook wants to merge 4 commits into
Draft
Conversation
Route build and finalize through ParallelExecutor instead of a directly held ForkJoinPool, so an embedding host can supply its own pool or run caller-runs (no work escaping to ForkJoinPool.commonPool()). Existing ForkJoinPool constructors are preserved; they now wrap ParallelExecutor.forkJoin(pool), so current callers are unaffected. The internal parallel-for sites move from executor.submit(...) to the ParallelExecutor.forEachInt / forEach entry points.
Let PQ, NVQ, and BQ encode/refine on a caller-supplied ParallelExecutor so quantization participates in the host's execution budget instead of the common pool. Additive: the existing ForkJoinPool entry points are kept as delegating overloads, so no current call path changes.
Carry the execution resources an embedder passes to jvector in one place -- a compute ParallelExecutor plus merge/io executors -- so the pool is supplied once and "no work escapes to the common pool" holds in a single spot. of(pool) bounds everything to a host pool; callerRuns() runs inline on the calling thread (the memtable-flush case). Wired here to the seams that exist on this branch: graph build (newBuilder) and PQ/NVQ train/refine/encode route through the compute executor. The compaction and parallel-writer wiring depends on the compactor's calling-convention conversion, which is illustrated in prose in doc/compaction-seam-conversion.md; the merge/io roles are carried so the note can reference them directly.
Add docs/compaction-seam-conversion.md: how OnDiskGraphIndexCompactor's calling conventions map onto the integration-robustness seams -- execution (Executor / EmbeddedExecutionContext), progress + throttle (ProgressLimiter), output (CompactionDestination / SeekableSink), PQ retrain and the parallel writer via the carried executors, plus the compactor-local memory-safety guards (drain-on-unwind, truncate-reused-outputs) that land with it. In the current lineage the compactor's seam-wiring is inseparable from the compaction-algorithm work, so it is described here rather than carried as code; this note is the target the clean compaction work should hit.
Contributor
|
Before you submit for review:
If you did not complete any of these, then please explain below. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is for illustrative purposes only. It should not be merged, so I'm setting it as Draft. We'll delete it when we're done with the review of the approach.
Purpose
Convert the general execution paths onto the embedding seams, and document the compactor conversion as an illustration of how the upstream integration robustness APIs might be used.
Why
The base PR added the embedding seams with no consumers. This PR shows how jvector's operations adopt them. Two categories:
Real code
GraphIndexBuilder→ParallelExecutor. Build and finalize route throughParallelExecutor(a host pool, or caller-runs so no work escapes to the common pool). ExistingForkJoinPoolconstructors are preserved — they now wrapParallelExecutor.forkJoin(pool)— so current callers are unaffected.ParallelExecutor.encode/refine/computeaccept aParallelExecutor; the existingForkJoinPoolentry points remain as delegating overloads. IncludesQuantizationCallerRunsTest, which verifies caller-runs output matches the pool-backed path.EmbeddedExecutionContext. A single carrier for the execution resources an embedder supplies — a computeParallelExecutorplusmerge/ioexecutors — so the pool is passed once and "no work escapes to the common pool" lives in one place.of(pool)bounds everything to a host pool;callerRuns()runs inline on the calling thread (the memtable-flush case). Build and PQ/NVQ train/refine/encode are wired through it.Documented —
docs/compaction-seam-conversion.mdbefore/afterfor each compactor calling convention, using the real integration-lineage signatures:Executorconstructor +taskWindowSizebound, defaulting to the shared pool only when none is supplied;setProgressLimiter(...), withonProgress(up) /acquire(down) at phase boundaries doubling as cancellation checkpoints;compact(CompactionDestination)with theTargetopen/commit/close lifecycle and host-owned footer viaSeekableSink.over(...);OnDiskParallelGraphIndexWriter.Builder.withExecutor(io);Note on
EmbeddedExecutionContextscopeIts compactor/writer/retrain factory methods (
newCompactor,newParallelWriter,retrainPQ) are intentionally omitted here — they call converted compactor APIs that don't exist yet — and are shown in the design note instead. Themerge/ioexecutor roles are still carried so the note can referencemergeExecutor()/ioExecutor()directly; those methods return as thin wiring once the compactor is converted.Testing
jvector-basecompiles at--release 11;QuantizationCallerRunsTestadded and compiles.Stats: 9 files, +934/−168.
Commits
graph: convert GraphIndexBuilder to the ParallelExecutor seamquantization: accept ParallelExecutor for PQ/NVQ/BQ encodegraph: EmbeddedExecutionContext as the single execution carrierdocs: illustrate the compactor's calling-convention conversion