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

8264732: Clean up LinkResolver::vtable_index_of_interface_method() #3346

Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -149,8 +149,7 @@ CallInfo::CallInfo(Method* resolved_method, Klass* resolved_klass, TRAPS) {
kind = CallInfo::vtable_call;
} else if (!resolved_klass->is_interface()) {
// A default or miranda method. Compute the vtable index.
index = LinkResolver::vtable_index_of_interface_method(resolved_klass,
_resolved_method);
index = LinkResolver::vtable_index_of_interface_method(resolved_klass, resolved_method);
assert(index >= 0 , "we should have valid vtable index at this point");

kind = CallInfo::vtable_call;
@@ -405,8 +404,7 @@ Method* LinkResolver::lookup_instance_method_in_klasses(Klass* klass,
return result;
}

int LinkResolver::vtable_index_of_interface_method(Klass* klass,
const methodHandle& resolved_method) {
int LinkResolver::vtable_index_of_interface_method(Klass* klass, Method* resolved_method) {

int vtable_index = Method::invalid_vtable_index;
Symbol* name = resolved_method->name();
@@ -1391,7 +1389,7 @@ void LinkResolver::runtime_resolve_virtual_method(CallInfo& result,

// do lookup based on receiver klass using the vtable index
if (resolved_method->method_holder()->is_interface()) { // default or miranda method
vtable_index = vtable_index_of_interface_method(resolved_klass, resolved_method);
vtable_index = vtable_index_of_interface_method(resolved_klass, resolved_method());
assert(vtable_index >= 0 , "we should have valid vtable index at this point");

selected_method = methodHandle(THREAD, recv_klass->method_at_vtable(vtable_index));
@@ -317,7 +317,7 @@ class LinkResolver: AllStatic {
static Method* resolve_static_call_or_null(const LinkInfo& link_info);
static Method* resolve_special_call_or_null(const LinkInfo& link_info);

static int vtable_index_of_interface_method(Klass* klass, const methodHandle& resolved_method);
static int vtable_index_of_interface_method(Klass* klass, Method* resolved_method);

// same as above for compile-time resolution; returns vtable_index if current_klass if linked
static int resolve_virtual_vtable_index (Klass* receiver_klass,
@@ -728,7 +728,7 @@ C2V_END

C2V_VMENTRY_0(jint, getVtableIndexForInterfaceMethod, (JNIEnv* env, jobject, jobject jvmci_type, jobject jvmci_method))
Klass* klass = JVMCIENV->asKlass(jvmci_type);
methodHandle method(THREAD, JVMCIENV->asMethod(jvmci_method));
Method* method = JVMCIENV->asMethod(jvmci_method);
Copy link
Contributor

@coleenp coleenp Apr 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason that resolved_method was a methodHandle is in case of redefinition, we need to know if code is referring to this version of the method so that it's not deallocated. It's enough for one of the callers to create a methodHandle but passing the methodHandle will guarantee it. I'm not sure why you needed to make this change.

Copy link
Author

@iwanowww iwanowww Apr 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I refactored getVtableIndexForInterfaceMethod and kept the handle in place while dereferencing it when passing into vtable_index_of_interface_method. Does it look better now?

Copy link
Contributor

@coleenp coleenp Apr 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, this is fine. Be careful if you have new callers for this function.

if (klass->is_interface()) {
JVMCI_THROW_MSG_0(InternalError, err_msg("Interface %s should be handled in Java code", klass->external_name()));
}