Skip to content

Add a memory-mapped RandomAccessReader using MemorySegment api - #296

Merged
jbellis merged 2 commits into
datastax:mainfrom
mdogan:memory-segment-reader
Apr 17, 2024
Merged

jbellis merged 2 commits into
datastax:mainfrom
mdogan:memory-segment-reader

Conversation

@mdogan

@mdogan mdogan commented Apr 16, 2024

Copy link
Copy Markdown
Contributor

I'd prefer this to be in jvector-twenty module. But --enable-preview flag is only allowed for the Java release version used to compile the code. When building with Java 22, --enable-preview is not allowed on twenty module because it builds for Java 20.

Closes #278

@mdogan
mdogan force-pushed the memory-segment-reader branch from 9ffd328 to edf50bb Compare April 16, 2024 14:58
@jbellis

jbellis commented Apr 16, 2024

Copy link
Copy Markdown
Contributor

Outstanding!

Can you wire this into ReaderSupplierFactory (replacing MMapReaderSupplier)? Not sure if reflection is needed given the module configuration.

@jkni

jkni commented Apr 16, 2024

Copy link
Copy Markdown
Contributor

Thank you @mdogan! I feel your pain here regarding module placement. We could adopt Maven Toolchains so that we could call out to a Java 20 compiler in jvector-twenty, allowing us to use preview features there. Any strong feelings about whether that's worth it? It only introduces marginal additional complexity in the build.

mdogan added 2 commits April 17, 2024 10:30
I'd prefer this to be in `jvector-twenty` module. But `--enable-preview` flag is only
allowed for the Java release version used to compile the code. When building with Java 22,
`--enable-preview` is not allowed on `twenty` module because it builds for Java 20.
@mdogan
mdogan force-pushed the memory-segment-reader branch from edf50bb to ee2fb37 Compare April 17, 2024 08:19
@mdogan

mdogan commented Apr 17, 2024

Copy link
Copy Markdown
Contributor Author

Outstanding!

Can you wire this into ReaderSupplierFactory (replacing MMapReaderSupplier)? Not sure if reflection is needed given the module configuration.

Done.

@mdogan

mdogan commented Apr 17, 2024 •

Copy link
Copy Markdown
Contributor Author

Thank you @mdogan! I feel your pain here regarding module placement. We could adopt Maven Toolchains so that we could call out to a Java 20 compiler in jvector-twenty, allowing us to use preview features there. Any strong feelings about whether that's worth it? It only introduces marginal additional complexity in the build.

Actually I don't care about Java 20 but maybe it would be useful for Java 21 users (since it's LTS). But your new native extensions are targeting 22, so this can remain in Java 22 too.

@jbellis

jbellis commented Apr 17, 2024

Copy link
Copy Markdown
Contributor

LGTM. Ship it!

@jbellis

jbellis commented Apr 17, 2024

Copy link
Copy Markdown
Contributor

(pushing the button since i want to put this in beta3)

@jbellis
jbellis merged commit e287eb0 into datastax:main Apr 17, 2024
@mdogan
mdogan deleted the memory-segment-reader branch April 18, 2024 06:12
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.

add a RandomAccessReader implementation for jdk 20+ using modern MMap or MemorySegment

3 participants