Skip to content

Use bulk writes instead of per-element writes in vector serialization#681

Open
r-devulap wants to merge 7 commits into
mainfrom
bulk-writes-memorySegment
Open

Use bulk writes instead of per-element writes in vector serialization#681
r-devulap wants to merge 7 commits into
mainfrom
bulk-writes-memorySegment

Conversation

@r-devulap

@r-devulap r-devulap commented Jun 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Replace inefficient element-by-element write loops with bulk write operations in MemorySegmentVectorProvider:

  • writeFloatVector: Extract underlying float array and use writeFloats() instead of looping with writeFloat()
  • writeByteSequence: Extract underlying byte array and use write() instead of looping with writeByte()

With highway as the SIMD backend #668 , the scalar writes show up as a bottleneck when constructing index on a AWS instance x8i.24xlarge (2-socket 96 core Intel GNR) .

Dataset Index Build Time Before (s) Index Build Time After (s)
openai-1536-1m 105.59 45.22
openai-3072-1m 164.63 102.75

@r-devulap
Use bulk writes instead of per-element writes in vector serialization
943d4bd 
Replace inefficient element-by-element write loops with bulk write
operations in MemorySegmentVectorProvider:

- writeFloatVector: Extract underlying float array and use writeFloats()
  instead of looping with writeFloat()
- writeByteSequence: Extract underlying byte array and use write()
  instead of looping with writeByte()
@github-actions

github-actions Bot commented Jun 17, 2026 •
edited by r-devulap
Loading

Copy link
Copy Markdown
Contributor

Before you submit for review:

  • Does your PR follow guidelines from CONTRIBUTIONS.md?
  • Did you summarize what this PR does clearly and concisely?
  • Did you include performance data for changes which may be performance impacting?
  • Did you include useful docs for any user-facing changes or features?
  • Did you include useful javadocs for developer oriented changes, explaining new concepts or key changes?
  • Did you trigger and review regression testing results against the base branch via Run Bench Main?
  • Did you adhere to the code formatting guidelines (TBD)
  • Did you group your changes for easy review, providing meaningful descriptions for each commit?
  • Did you ensure that all files contain the correct copyright header?

If you did not complete any of these, then please explain below.

tlwillke
tlwillke requested changes Jun 17, 2026
View reviewed changes

@tlwillke tlwillke left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

writeFloatVector is fine. writeByteSequence has a bug. You are ignoring slice offsets. MemorySegmentByteSequence supports slicing, but .heapBase().get() returns the original unsliced array. I have suggested a change that fixes this and added minimal testing.

Once this is addressed and the tests pass, it is good to go.

On performance, while I appreciate the end-to-end benchmarking and consider it necessary, I would also like to see a unit-level microbenchmark isolating writeFloatVector and writeByteSequence to quantify the exact bulk-write speedup.

Comment thread jvector-native/src/main/java/io/github/jbellis/jvector/vector/MemorySegmentVectorProvider.java
@tlwillke tlwillke added the performance improvement A contribution that aims to improve library performance, possibly along with functionality. label Jun 17, 2026
@r-devulap
Use .asByteBuffer() instead of .heapBase().get() for MemorySegmentByt…
df2aed3 
…eSequence

,heapBase().get() ignores slicing and returns the base of the original
ByteArray.
@r-devulap

r-devulap commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

Had to move MemorySegmentVectorProviderTest to jvector-native module, it fails to run on JAVA 20.

@r-devulap r-devulap force-pushed the bulk-writes-memorySegment branch from 4d5f59d to 3c0057b Compare June 18, 2026 05:53
@r-devulap

r-devulap commented Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Added microbenchmarks. Let me know if that is what you what you were looking for. Here is the comparison with main branch on a x8i.24xlarge (96 core Intel GNR)

| Benchmark        | Length | Mode  | Cnt | Branch Score | Main Score  | Speedup (x) | Units |
|------------------|--------|-------|-----|--------------|-------------|-------------|-------|
| writeByteVector  | 512    | thrpt | 10  | 6029596.287  | 3904220.037 | 1.54x       | ops/s |
| writeByteVector  | 1024   | thrpt | 10  | 3230877.500  | 1924332.336 | 1.68x       | ops/s |
| writeByteVector  | 1536   | thrpt | 10  | 2076349.428  | 1273820.153 | 1.63x       | ops/s |
| writeFloatVector | 512    | thrpt | 10  | 1996524.347  | 1486490.747 | 1.34x       | ops/s |
| writeFloatVector | 1024   | thrpt | 10  | 978881.406   | 520671.373  | 1.88x       | ops/s |
| writeFloatVector | 1536   | thrpt | 10  | 672933.883   | 438508.050  | 1.53x       | ops/s |

tlwillke
tlwillke approved these changes Jun 18, 2026
View reviewed changes

@tlwillke tlwillke left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the fix and adding the microbenchmarks. Looks good.

MarkWolters
MarkWolters approved these changes Jun 18, 2026
View reviewed changes

@MarkWolters MarkWolters left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks fine to me

jshook
jshook approved these changes Jun 18, 2026
View reviewed changes

@jshook jshook left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good

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

Reviewers

@jshook jshook jshook approved these changes

@tlwillke tlwillke tlwillke approved these changes

@MarkWolters MarkWolters MarkWolters approved these changes

@ashkrisk ashkrisk Awaiting requested review from ashkrisk ashkrisk is a code owner

Assignees

@r-devulap r-devulap

Labels

performance improvement A contribution that aims to improve library performance, possibly along with functionality.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@r-devulap @jshook @tlwillke @MarkWolters