Skip to content

Remove larray#13545

Closed
suddendust wants to merge 2 commits intoapache:masterfrom
suddendust:remove_larray
Closed

Remove larray#13545
suddendust wants to merge 2 commits intoapache:masterfrom
suddendust:remove_larray

Conversation

@suddendust
Copy link
Contributor

@suddendust suddendust commented Jul 4, 2024

Removing larray as it is not used anymore and it fails the builds on ARM64 as this library doesn't support MacOS/ARM64. Further, this project is now in maintenance mode.

@suddendust suddendust closed this Jul 4, 2024
@suddendust suddendust reopened this Jul 4, 2024
@suddendust suddendust closed this Jul 4, 2024
@siddharthteotia
Copy link
Contributor

Do existing users of Larray automatically get defaulted to the new implementation of > 2GB buffers (added sometime ago) ? I don't recollect exactly how the new mechanism of supporting larger than 2GB buffers (foreign memory API IIRC) is wired in. Can @suddendust or may be @gortiz confirm ?

At LinkedIn, we do have use cases where the index buffers are larger than 2GB and they depend on LArrayBuffer support for the same. So I would like to ensure first that we don't break.

Overall, I am aligned that this library should be removed from the code base and existing users (like us) should start using the new implementation.

@gortiz
Copy link
Contributor

gortiz commented Jul 5, 2024

The alternative implementation uses sun.misc.Unsafe. It is very easy to create a new implementation on top of Foreign memory API, but it has two main issues:

  • Foreign memory API is only supported in Java 21 with preview enabled. It is ready to use in production in Java 22.
  • Our CI pipelines right now do not support Java 21 only code. I'm trying to add that support in order add the ability to execute multi-stage queries using virtual threads, but it is a bit messy to deal with the CI right now.

We have been using the alternative based on sun.misc.Unsafe for a while with no issues. The only thing that is required is to open sun.misc.Unsafe in the newer versions of Java, but IIRC that is already a requirement of Pinot (ie in order to manually release byte buffers).

Anyway, we can keep LArray for a while. We may for example first enable our alternative by default in all JVMS for one release and in the next one remove LArray. What do you think?

@gortiz gortiz mentioned this pull request Jul 5, 2024
2 tasks
@gortiz
Copy link
Contributor

gortiz commented Jul 17, 2024

I've just open #13639 to disable LArray by default as a first step to end up removing it completely

@siddharthteotia
Copy link
Contributor

Thanks @gortiz. I agree at a high level but may have concerns on the approach. Let me chime in on the other thread you started

@gortiz
Copy link
Contributor

gortiz commented Mar 21, 2025

I'm finally removing LArray in #15342

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.

3 participants