8256368: Avoid repeated upcalls into Java to re-resolve MH/VH linkers/invokers #1453
Conversation
|
|
iwanowww
changed the title
|
/label add hotspot-compiler |
|
@iwanowww |
|
@iwanowww This change now passes all automated pre-integration checks. 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 36 new 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 this automatic rebasing, please check the documentation for the /integrate command for further details.
|
| bool LinkResolver::resolve_previously_linked_invokehandle(CallInfo& result, const LinkInfo& link_info, const constantPoolHandle& pool, int index, TRAPS) { | ||
| int cache_index = ConstantPool::decode_cpcache_index(index, true); | ||
| ConstantPoolCacheEntry* cpce = pool->cache()->entry_at(cache_index); | ||
| if (!cpce->is_f1_null()) { |
There was a problem hiding this comment.
If f1 is non-null any racing resolution is complete. Can you double check if that's also the case for indy_resolution_failed?
There was a problem hiding this comment.
My reading is that a non-null f1 means any resolution races is complete and writes to indy_resolution_failed (part of flags) is done before writing f1, see comments in and logic in ConstantPoolCacheEntry::set_method_handle_common. It thus seem consistent and correct to resolve successfully here, no?
|
@iwanowww This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
|
@iwanowww This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! |
Method linkage of
invokehandleinstructions involve an upcall into Java (MethodHandleNatives::linkMethod), but the result is not cached. Unless the upcall behaves idempotently (which is highly desirable, but not guaranteed), repeated invokehandle resolution attempts enter a vicious cycle in tiered mode: switching to a higher tier involves call site re-resolution in generated code, but re-resolution installs a fresh method which puts execution back into interpreter.(Another prerequisite is no inlining through a
invokehandlewhich doesn't normally happen in practice - relevant methods are marked w/@ForceInline- except some testing modes,-Xcompin particular.)Proposed fix is to inspect
ConstantPoolCachefirst. Previous resolution attempts from interpreter and C1 records their results there and it stabilises the execution.Testing:
Progress
Testing
Failed test tasks
Issue
Reviewers
Download
$ git fetch https://git.openjdk.java.net/jdk pull/1453/head:pull/1453$ git checkout pull/1453