This repository was archived by the owner on Dec 12, 2022. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 8
Add a ByteBuffer based implementation of Buffer #37
Merged
Conversation
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
Motivation: We need a new implementation of our new API that supports Java 11, since that is what Netty 5 will most likely baseline on. We also need an implementation that does not rely on Unsafe. This leaves us with ByteBuffer as the underlying currency of memory. Modification: - Add a NioBuffer implementation and associated supporting classes. - The entry-point for this is a new MemoryManagers API, which is used to pick the implementation and provide the on-/off-heap MemoryManager implementations. - Add a mechanism to configure/override which MemoryManagers implementation to use. - The MemoryManagers implementations are service-loadable, so new ones can be discovered at runtime. - The existing MemorySegment based implementation also get a MemoryManagers implementation. - Expand the BufferTest to include all combinations of all implementations. We now run 360.000 tests in BufferTest. - Some common infrastructure, like ArcDrop, is moved to its own package. - Add a module-info.java to control the service loading, and the visibility in the various packages. - Some pom.xml file updates to support our now module based project. Result: We have an implementation that should work on Java 11, but we currently don't build or test on 11. More work needs to happen before that is a reality.
Draft for the time being because I think we need some Netty changes before we can access the |
normanmaurer
approved these changes
Mar 18, 2021
The netty-buffer test-jar dependency was causing some problems, but this works around that.
chrisvest
commented
Mar 19, 2021
These two are really only needed for test-compile, but the maven-compiler-plugin cannot express that | ||
--> | ||
<arg>--patch-module</arg> | ||
<arg>io.netty.buffer=${io.netty:netty-buffer:test-jar:tests}</arg> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has to be placed in compilerArgs
instead of the various testCompiler*
configurations, because the latter cannot correctly handle space-separated argument-value pairs.
This is a maven-compiler-plugin
limitation, that apache/maven-compiler-plugin#27 will hopefully solve.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Motivation:
We need a new implementation of our new API that supports Java 11, since that is what Netty 5 will most likely baseline on.
We also need an implementation that does not rely on Unsafe.
This leaves us with ByteBuffer as the underlying currency of memory.
Modification:
Result:
We have an implementation that should work on Java 11, but we currently don't build or test on 11.
More work needs to happen before that is a reality.