-
Notifications
You must be signed in to change notification settings - Fork 61
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
JDK-8243669: Improve library loading for Panama libraries #132
Conversation
👋 Welcome back mcimadamore! A progress list of the required criteria for merging this PR into |
Webrevs
|
Mailing list message from Samuel Audet on panama-dev: If reference counting gets used for both memory allocations and library Samuel 2020?4?27?(?) 23:32 Maurizio Cimadamore <mcimadamore at openjdk.java.net>:
|
Mailing list message from Maurizio Cimadamore on panama-dev: On 27/04/2020 23:13, Samuel Audet wrote:
There's no _explicit_ reference counting mechanism anywhere - not in the Maurizio
|
Mailing list message from Samuel Audet on panama-dev: On 4/28/20 10:39 AM, Maurizio Cimadamore wrote:
I understand that there is no "explicit reference counting", but it is Samuel |
@mcimadamore This change now passes all automated pre-integration checks, type
Since the source branch of this PR was last updated there have been 4 commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid automatic rebasing, please merge ➡️ To integrate this PR with the above commit message to the |
Mailing list message from Samuel Audet on panama-dev: On 4/28/20 11:15 AM, Samuel Audet wrote:
After rereading your proposal a second time, it's clear that you are Well, I do agree that we shouldn't be doing reference counting manually, The idea with a standard API for reference counting would be to offer a It works well, and I do see the JDK offering something like that, but Do you disagree and if so, why? Samuel |
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.
Looks good, but one test is failing on Windows. Left an inline comment.
src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/LibrariesHelper.java
Outdated
Show resolved
Hide resolved
Mailing list message from Maurizio Cimadamore on panama-dev: On 28/04/2020 09:24, Samuel Audet wrote:
My take on scope is that they work generally well - but, to make them Thread A accessing a pointer while thread B is 'releasing' it, where A So, in my mental model, refcounts, scopes are _tools. If the _goal_ is As for the claim that library and memory resources are in the same Maurizio
|
* enhanced test to also try loading same library from multiple class loaders
Mailing list message from Samuel Audet on panama-dev: On 4/28/20 8:56 PM, Maurizio Cimadamore wrote:
Yes, that's the basic idea, but the point is that we can do everything
Right, it's not perfect, but I think these kinds of issues are solvable,
My point is that this can all be part of a standard API to deal with all Samuel |
I like this simpler solution to let GC manage the native library unloading as in |
Mailing list message from Maurizio Cimadamore on panama-dev:
This is the very central issue we're trying to address with memory * retaining access performances It's not a theoretical problem. If you want to make your JavaCPP code Other solutions "include" something like reference counting, but not the While something like that might be made to work (we had something All this to say what I was trying to say before: wrapping up So, I don't see a lot of value for providing such an abstraction in the Maurizio |
/integrate |
@mcimadamore The following commits have been pushed to foreign-abi since your change was applied:
Your commit was automatically rebased without conflicts. Pushed as commit 7b6d993. |
Mailing list message from Samuel Audet on panama-dev: On 4/30/20 7:07 PM, Maurizio Cimadamore wrote:
Thank you for bearing with me, but I must seriously be missing Main Thread: Thread 1: Thread 2: We have only one instance of the object, while locks, if they are Samuel |
Mailing list message from Maurizio Cimadamore on panama-dev: On 01/05/2020 10:57, Samuel Audet wrote:
Are you now proposing a _language_ feature - not just an API? Maurizio
|
Mailing list message from Samuel Audet on panama-dev: On 5/1/20 7:34 PM, Maurizio Cimadamore wrote:
Yes, that's what I said above. Do you agree this could work? I'm not saying that it's going to be easy, but if the Java community Samuel |
Mailing list message from Maurizio Cimadamore on panama-dev: On 02/05/2020 10:34, Samuel Audet wrote:
Changing the language to support reference counting in a language that On top of that, moving the problem onto the language typically amounts And, despite all that, there are still cases where the close() method in To which you might say "why don't you just do all that?" - and the Pointer p .... This is a relatively pleasing API - which has the added benefit that it Maurizio
|
Mailing list message from Samuel Audet on panama-dev: Hi, Maurizio, On 5/5/20 7:32 PM, Maurizio Cimadamore wrote:
That actually looks good, I agree, but I still don't see why that kind It sounds like you're saying that this pattern is so easy to implement That's basically my stance. I'm not trying to push solutions, I'm just Again, I'm not saying that all of this is going to be easy to figure Samuel |
Mailing list message from Maurizio Cimadamore on panama-dev: On 05/05/2020 23:56, Samuel Audet wrote:
I think we're going in circles. The PR summary explains why we can't use Even with the API I proposed like this: Pointer p .... There is a big caveat; this idiom only guarantees 100% safety is the But I simply don't buy that what works for memory works for libraries. I'm all for unification and minimizing abstractions... where it makes Maurizio |
Mailing list message from Maurizio Cimadamore on panama-dev: On 05/05/2020 23:56, Samuel Audet wrote:
We are aware of that, and nobody has really mentioned that said devices https://gist.github.com/mcimadamore/128ee904157bb6c729a10596e69edffd Now, replace mmap/munmap with cudaMalloc/cudaFree and you will have a Of course the memory access API is a building block - together with ABI Maurizio |
Mailing list message from Samuel Audet on panama-dev: On 5/6/20 8:36 AM, Maurizio Cimadamore wrote:
I'm sorry if I'm making absurd claims about information that you're not
That looks like a good starting point, yes. Are saying that this is Let's assume this is going to be all public. The next thing that worries
Right, that's how I see it, but your lack of reply to my query about the Samuel |
Mailing list message from Maurizio Cimadamore on panama-dev: On 09/05/2020 03:16, Samuel Audet wrote:
I didn't see that comment. In general you can attach whatever index In your example the filtering function could be something like this @OverRide So, assuming you have a plain indexed var handle whose only coordinate VarHandle(MemoryAddress, long) to VarHandle(MemoryAddress, long, long, long) where, on each access, the above function will be computed, yield a long Maurizio
|
Mailing list message from Samuel Audet on panama-dev: On 5/11/20 7:11 PM, Maurizio Cimadamore wrote:
Ok, good to hear that. Please do bounce off your ideas about that here
What about offering an option to do more or less the same thing as what MemorySegment segment = MemorySegment.allocateCloseableNative(...); MemorySegment segment = MemorySegment.allocateLongLivedNative(...); Not exactly perfect, but still a step in the right direction.
Ok, thank you, I've replied there: It would be nice if you were able to make more information public about Samuel |
Mailing list message from Maurizio Cimadamore on panama-dev: I'm closing down this thread, sorry. We're going in circles and this has absolutely nothing to do with the Maurizio On 12/05/2020 02:54, Samuel Audet wrote:
|
Mailing list message from Samuel Audet on panama-dev: I don't feel that we're going in circles, I thought we were making Samuel On 5/12/20 7:13 PM, Maurizio Cimadamore wrote:
|
The code for loading native libraries has recently been cleaned up so as to allow for same library to be loaded by multiple loaders, in case the library being loaded is NOT a JNI library - see JDK-8240975.
This patch relaxes the Panama library loading mechanism so that the classloader restriction is dropped; in fact, Panama library loading is now orthogonal from the class loader in which the loading operation occurs.
The main issue with this enhancement is to decide how libraries should be unloaded, given that same library might be referred to by many different lookup objects in many different threads.
If we aim for fully explicit library unloading (e.g.
LibraryLookup::close
) this raises similar issues to those we have for foreign memory access: we now have a library lookup which can be closed while a method handle is operating on an address generated by it in some other thread.We could solve the problem in the same way we solved the memory segment problem - that is, making library lookup objects thread-confined, and have each address be invalidated when a lookup is closed (but then looked up addresses are only usable withing the confinement thread). While doable, this seems to go against how clients will use
SystemABI
to generate native method handles, where they will probably want to stash a bunch of method handles in static final constants - and if these handles depend on a confined address, it means they can't be shared.A saner solution is to let the GC manage library unloading - that is, we setup a reference counter with each loaded library; the more lookups are created from the same underlying native library, the more the counter is incremented; as lookup instances become unreachable, the counter is decremented. When the counter reaches zero, the library is also unloaded.
To prevent races between library loading/unloading, the two routines in charge of loading/unloading have been marked as synchronized. This also means that the lookup on the
nativeLibraries
instance has to be performed inside the synchronized block (we don't want to accidentally try to use aNativeLibrary
instance which has concurrently been unloaded by the cleaner).This is a simple strategy, but also very effective: the lifetime of a LibraryLookup controls that of the native library it is associated with. In addition, all memory addresses generated by a lookup keep a strong reference to the lookup - and a native method handle generated by a
SystemABI::downcallHandle
call will also keep a strong reference to the address it refers to. Which means that if you store a method handle in a static final field, you don't have to worry about theLibraryLookup
becoming unreachable, as it will be kept alive by the method handles.Progress
Issue
Reviewers
Download
$ git fetch https://git.openjdk.java.net/panama-foreign pull/132/head:pull/132
$ git checkout pull/132