8276648: Downcall stubs are never freed #651
Conversation
|
👋 Welcome back jvernee! A progress list of the required criteria for merging this PR into |
| makeInvoker(methodType, abi, argMoves, returnMoves, needsReturnBuffer)); | ||
|
|
||
| return new NativeEntryPoint(methodType, invoker); | ||
| return INVOKER_CACHE.get(key, k -> { |
There was a problem hiding this comment.
This uses a combination of soft references (cache) and phantom references (cleaner). So, cache entries will be evicted when the NEP is softly reachable - meaning that the GC will clear the entries when under pressure. But the cleaner won't be able to run until the entry is evicted (because the entry has a soft reference to the NEP, and to be phantom-reachable you need NOT to be soft-reachable). Assuming that's what you want, then it looks ok. E.g. the cleaner here will "just" make sure that some action is executed when the entry is evicted.
There was a problem hiding this comment.
Yes, that is the intent. We want to keep the downcall stub around until after the NEP is evicted from the cache. The cleanup is just there to make sure the VM stub is freed as well, and it is tied to the reachability of the associated NEP, which is strongly reachable from a downcall method handle.
|
@JornVernee This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 2 new commits pushed to the
Please see this link for an up-to-date comparison between the source branch of this pull request and the ➡️ To integrate this PR with the above commit message to the |
|
/integrate |
|
Going to push as commit 4115fcb.
Your commit was automatically rebased without conflicts. |
|
@JornVernee Pushed as commit 4115fcb. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
Hi,
The current code adds native invokers (downcall stubs) to a CocurrentHashMap cache, and they stay there until the application exits.
This patch replaces that cache with a SoftReferenceCache (brought to the top level from AbstractLinker), which allows at least the values to be evicted when they become unreachable.
Those value, which are NativeEntryPoint instances, are also registered with a cleaner, whose cleanup action will free the stub from the code cache.
Thanks,
Jorn
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.java.net/panama-foreign pull/651/head:pull/651$ git checkout pull/651Update a local copy of the PR:
$ git checkout pull/651$ git pull https://git.openjdk.java.net/panama-foreign pull/651/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 651View PR using the GUI difftool:
$ git pr show -t 651Using diff file
Download this PR as a diff file:
https://git.openjdk.java.net/panama-foreign/pull/651.diff