Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Foreign Memory API, introduced in Java 22 #12809

Open
gortiz opened this issue Apr 8, 2024 · 6 comments
Open

Use Foreign Memory API, introduced in Java 22 #12809

gortiz opened this issue Apr 8, 2024 · 6 comments
Labels
beginner-task Small task for new contributors to ramp up help wanted

Comments

@gortiz
Copy link
Contributor

gortiz commented Apr 8, 2024

This is a sibling of #12810

TL;DR:

Create a new buffer implementation that uses official Java Foreign Memory API. See JEP-454.

As a requirement, this addition should:

  • Not assume Pinot is compiled with Java 21 or 22
  • Not be loaded if:
    • Option 1 (safer): The runtime is <= 21
    • Option 2: The runtime is <= 20 or equal to 21 and preview is not enabled.

Context

As we may know Java buffers length has been limited to 2GBs. In Apache Pinot we decided to skip that limit by using LArray, a third party buffer library that uses JNI to create larger buffers. That library is not maintained anymore (see xerial/larray#75 (comment) or https://github.com/xerial/larray/issues/80).\

This library has several problems, including:

  • It may produce segmentation faults or buffer overflows (there is no index check when using it).
  • It doesn't work in Mac using arm architecture.
  • It doesn't support Java >= 15.

In order to solve these problems, we introduced a new buffer implementation called unsafe. The name comes from the fact that uses sun.misc.Unsafe, but AFAIK it is safer than LArray. It works and that is what we use by default if at runtime we detect we are using Java >= 15.

But in March 2024, Java 22 was released. This new Java version includes JEP-454 Foreign Memory API, which includes new APIs that solve the original issue (be able to index buffers with 64 bits) while providing a more modern, safer, tested and stable API.

When I've added unsafe buffers, I've also tried Foreign Memory API (in preview state) and it was super simple to create a buffer implementation with that. The single problem I had was maven related. Given this API can only be used in Java 22 (or maybe 21 if preview features are enabled), we can only compile that code with that Java version and we need to be sure that code is not loaded if Java < 21 is used. At the time I didn't had time to deal with these two issues and therefore I'm creating this issue as a reminder for my future self or, alternative, as a birdcall in case someone else wants to learn more about Maven and/or Foreign Memory APi.

Proposed solution

  • Create a new Maven sub-project
  • Only compile sources of this project when using Java 22 by setting a default profile where Java sources are empty
  • This project should use multi release versions, adding the sources under Java version 22. By doing that we can be sure code that runs with Java <= 21 do not see these classes.
  • Be sure we publish this project to Maven (usually we publish code compiled with Java 11)
@gortiz gortiz added help wanted beginner-task Small task for new contributors to ramp up labels Apr 8, 2024
@aditya0811
Copy link
Contributor

Hello @gortiz I can try looking into this. I primarily see two parts to this.
a) Implementing buffer implementation using Foreign API by creating maven sub project.
b) Configuring classes to be loaded for this sub-project with the detected Java. For now I will consider we are going with option 1.

For a) I will start going through what we have implemented currently in package : org.apache.pinot.segment.spi.memory.unsafe.

Let me know if there are any specifics we want to start looking into initially.

@gortiz
Copy link
Contributor Author

gortiz commented Apr 15, 2024

I think there is a third part which may be more complex: How to compile a maven subproject that requires Java 22 while not affecting the rest of the project (which requires Java 11 or 8).

About point A, I already created one implementation at the same time I've created the unsafe buffers, but ended up removing it to do not have to deal with the third point I mentioned. The code is already in git (see for example this commit). Feel free to get inspiration from it

@aditya0811
Copy link
Contributor

ohk, in this case I will start looking for

How to compile a maven subproject that requires Java 22 while not affecting the rest of the project (which requires Java 11 or 8).

@aditya0811
Copy link
Contributor

@gortiz why cant we use reflection as discussed in para 1 here? And depending on java runtime invoke appropriate methods.

@gortiz
Copy link
Contributor Author

gortiz commented Apr 29, 2024

why cant we use reflection as discussed in para 1 here?

What do you mean with that? To always call the methods using reflection? That sounds unsafe for sure and probably not very efficient. And the buffer code is part of the Pinot hot path, so it should be as efficient as possible. By unsafe I mean that if for whatever reason we end up even loading the class that is compiled with Java > 11 we will end up with an unexpected error in runtime.

Therefore we cannot add code compiled with Java 22 in the main source code because we need our executables to be compatible with Java 11 (and the driver with Java 8). The clean solution to do that is multi-module projects. You can see in Pinot distributables that our dependencies are already doing that (including for example roaringbitmap).

I think the solution named as single-project in maven documentation should be fine. Specifically, we need a project with no code in the main source folder (where Java 11 compatible code must stay) and then another folder for Java 22 should contain all the new code. Then with Maven we would only compile Java 22 code if the JRE running the compilation process is at least Java 22.

@aditya0811
Copy link
Contributor

ohk, let me try the solution in single project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beginner-task Small task for new contributors to ramp up help wanted
Projects
None yet
Development

No branches or pull requests

2 participants