8217660: Refactor module related locked_create_entry_or_null() functions

Remove function return values and add functions that create entries without doing unneeded lookups.

Reviewed-by: redestad, lfoltan
This commit is contained in:
Harold Seigel 2019-01-24 09:38:50 -05:00
parent e4da9cdab0
commit ac22352b49
7 changed files with 66 additions and 57 deletions

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1997, 2019, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -1724,7 +1724,7 @@ void ClassLoader::create_javabase() {
{
MutexLocker ml(Module_lock, THREAD);
ModuleEntry* jb_module = null_cld_modules->locked_create_entry_or_null(Handle(),
ModuleEntry* jb_module = null_cld_modules->locked_create_entry(Handle(),
false, vmSymbols::java_base(), NULL, NULL, null_cld);
if (jb_module == NULL) {
vm_exit_during_initialization("Unable to create ModuleEntry for " JAVA_BASE_NAME);

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -398,23 +398,22 @@ void ModuleEntryTable::add_entry(int index, ModuleEntry* new_entry) {
Hashtable<Symbol*, mtModule>::add_entry(index, (HashtableEntry<Symbol*, mtModule>*)new_entry);
}
ModuleEntry* ModuleEntryTable::locked_create_entry_or_null(Handle module_handle,
bool is_open,
Symbol* module_name,
Symbol* module_version,
Symbol* module_location,
ClassLoaderData* loader_data) {
assert(module_name != NULL, "ModuleEntryTable locked_create_entry_or_null should never be called for unnamed module.");
// Create an entry in the class loader's module_entry_table. It is the
// caller's responsibility to ensure that the entry has not already been
// created.
ModuleEntry* ModuleEntryTable::locked_create_entry(Handle module_handle,
bool is_open,
Symbol* module_name,
Symbol* module_version,
Symbol* module_location,
ClassLoaderData* loader_data) {
assert(module_name != NULL, "ModuleEntryTable locked_create_entry should never be called for unnamed module.");
assert(Module_lock->owned_by_self(), "should have the Module_lock");
// Check if module already exists.
if (lookup_only(module_name) != NULL) {
return NULL;
} else {
ModuleEntry* entry = new_entry(compute_hash(module_name), module_handle, is_open, module_name,
module_version, module_location, loader_data);
add_entry(index_for(module_name), entry);
return entry;
}
assert(lookup_only(module_name) == NULL, "Module already exists");
ModuleEntry* entry = new_entry(compute_hash(module_name), module_handle, is_open, module_name,
module_version, module_location, loader_data);
add_entry(index_for(module_name), entry);
return entry;
}
// lookup_only by Symbol* to find a ModuleEntry.

View File

@ -236,14 +236,14 @@ public:
return (ModuleEntry*)Hashtable<Symbol*, mtModule>::bucket(i);
}
// Create module in loader's module entry table, if already exists then
// return null. Assume Module_lock has been locked by caller.
ModuleEntry* locked_create_entry_or_null(Handle module_handle,
bool is_open,
Symbol* module_name,
Symbol* module_version,
Symbol* module_location,
ClassLoaderData* loader_data);
// Create module in loader's module entry table. Assume Module_lock
// has been locked by caller.
ModuleEntry* locked_create_entry(Handle module_handle,
bool is_open,
Symbol* module_name,
Symbol* module_version,
Symbol* module_location,
ClassLoaderData* loader_data);
// Only lookup module within loader's module entry table. The table read is lock-free.
ModuleEntry* lookup_only(Symbol* name);
@ -253,7 +253,10 @@ public:
// Special handling for java.base
static ModuleEntry* javabase_moduleEntry() { return _javabase_module; }
static void set_javabase_moduleEntry(ModuleEntry* java_base) { _javabase_module = java_base; }
static void set_javabase_moduleEntry(ModuleEntry* java_base) {
assert(_javabase_module == NULL, "_javabase_module is already defined");
_javabase_module = java_base;
}
static bool javabase_defined() { return ((_javabase_module != NULL) &&
(_javabase_module->module() != NULL)); }

View File

@ -208,10 +208,9 @@ static void define_javabase_module(jobject module, jstring version,
// loop through and add any new packages for java.base
for (int x = 0; x < pkg_list->length(); x++) {
// Some of java.base's packages were added early in bootstrapping, ignore duplicates.
PackageEntry* pkg =
package_table->locked_create_entry_or_null(pkg_list->at(x),
ModuleEntryTable::javabase_moduleEntry());
assert(pkg != NULL || package_table->locked_lookup_only(pkg_list->at(x)) != NULL,
package_table->locked_create_entry_if_not_exist(pkg_list->at(x),
ModuleEntryTable::javabase_moduleEntry());
assert(package_table->locked_lookup_only(pkg_list->at(x)) != NULL,
"Unable to create a " JAVA_BASE_NAME " package entry");
// Unable to have a GrowableArray of TempNewSymbol. Must decrement the refcount of
// the Symbol* that was created above for each package. The refcount was incremented
@ -402,20 +401,17 @@ void Modules::define_module(jobject module, jboolean is_open, jstring version,
// Add the module and its packages.
if (!dupl_modules && existing_pkg == NULL) {
// Create the entry for this module in the class loader's module entry table.
ModuleEntry* module_entry = module_table->locked_create_entry_or_null(module_handle,
if (module_table->lookup_only(module_symbol) == NULL) {
// Create the entry for this module in the class loader's module entry table.
ModuleEntry* module_entry = module_table->locked_create_entry(module_handle,
(is_open == JNI_TRUE), module_symbol,
version_symbol, location_symbol, loader_data);
assert(module_entry != NULL, "module_entry creation failed");
if (module_entry == NULL) {
dupl_modules = true;
} else {
// Add the packages.
assert(pkg_list->length() == 0 || package_table != NULL, "Bad package table");
PackageEntry* pkg;
for (int y = 0; y < pkg_list->length(); y++) {
pkg = package_table->locked_create_entry_or_null(pkg_list->at(y), module_entry);
assert(pkg != NULL, "Unable to create a module's package entry");
package_table->locked_create_entry(pkg_list->at(y), module_entry);
// Unable to have a GrowableArray of TempNewSymbol. Must decrement the refcount of
// the Symbol* that was created above for each package. The refcount was incremented
@ -425,6 +421,8 @@ void Modules::define_module(jobject module, jboolean is_open, jstring version,
// Store pointer to ModuleEntry record in java.lang.Module object.
java_lang_Module::set_module_entry(module_handle(), module_entry);
} else {
dupl_modules = true;
}
}
} // Release the lock

View File

@ -211,17 +211,22 @@ void PackageEntryTable::add_entry(int index, PackageEntry* new_entry) {
Hashtable<Symbol*, mtModule>::add_entry(index, (HashtableEntry<Symbol*, mtModule>*)new_entry);
}
// Create package in loader's package entry table and return the entry.
// If entry already exists, return null. Assume Module lock was taken by caller.
PackageEntry* PackageEntryTable::locked_create_entry_or_null(Symbol* name, ModuleEntry* module) {
// Create package entry in loader's package entry table. Assume Module lock
// was taken by caller.
void PackageEntryTable::locked_create_entry(Symbol* name, ModuleEntry* module) {
assert(Module_lock->owned_by_self(), "should have the Module_lock");
// Check if package already exists. Return NULL if it does.
if (locked_lookup_only(name) != NULL) {
return NULL;
} else {
PackageEntry* entry = new_entry(compute_hash(name), name, module);
add_entry(index_for(name), entry);
return entry;
assert(locked_lookup_only(name) == NULL, "Package entry already exists");
PackageEntry* entry = new_entry(compute_hash(name), name, module);
add_entry(index_for(name), entry);
}
// Create package entry in loader's package entry table if it does not already
// exist. Assume Module lock was taken by caller.
void PackageEntryTable::locked_create_entry_if_not_exist(Symbol* name, ModuleEntry* module) {
assert(Module_lock->owned_by_self(), "should have the Module_lock");
// Check if package entry already exists. If not, create it.
if (locked_lookup_only(name) == NULL) {
locked_create_entry(name, module);
}
}

View File

@ -246,9 +246,13 @@ public:
return (PackageEntry*)Hashtable<Symbol*, mtModule>::bucket(i);
}
// Create package in loader's package entry table and return the entry.
// If entry already exists, return null. Assume Module lock was taken by caller.
PackageEntry* locked_create_entry_or_null(Symbol* name, ModuleEntry* module);
// Create package entry in loader's package entry table. Assume Module
// lock was taken by caller.
void locked_create_entry(Symbol* name, ModuleEntry* module);
// Create package entry in loader's package entry table if it does not
// already exist. Assume Module lock was taken by caller.
void locked_create_entry_if_not_exist(Symbol* name, ModuleEntry* module);
// Lookup Package with loader's package entry table, add it if not found.
// This will acquire the Module lock.

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -42,7 +42,7 @@ public class CheckForProperDetailStackTrace {
public static String stackTraceDefault =
".*Hashtable.*allocate_new_entry.*\n" +
".*ModuleEntryTable.*new_entry.*\n" +
".*ModuleEntryTable.*locked_create_entry_or_null.*\n" +
".*ModuleEntryTable.*locked_create_entry.*\n" +
".*Modules.*define_module.*\n";
/* The stack trace we look for on Solaris and Windows slowdebug builds. For some
@ -50,12 +50,12 @@ public class CheckForProperDetailStackTrace {
public static String stackTraceAllocateHeap =
".*AllocateHeap.*\n" +
".*ModuleEntryTable.*new_entry.*\n" +
".*ModuleEntryTable.*locked_create_entry_or_null.*\n" +
".*ModuleEntryTable.*locked_create_entry.*\n" +
".*Modules.*define_module.*\n";
/* A symbol that should always be present in NMT detail output. */
private static String expectedSymbol =
"locked_create_entry_or_null";
"locked_create_entry";
public static void main(String args[]) throws Exception {
ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(