8077203: Avoid unnecessary stripping of package names from FQN's in is_same_class_package() methods
authorhseigel
Thu, 20 Jul 2017 11:01:20 -0400
changeset 46697 2fdbdc5e0765
parent 46696 b5771fe1620a
child 46698 fa625dca9270
8077203: Avoid unnecessary stripping of package names from FQN's in is_same_class_package() methods Summary: Extract package name from instance klass's package entry record Reviewed-by: redestad, coleenp, lfoltan
hotspot/src/share/vm/classfile/classFileParser.cpp
hotspot/src/share/vm/oops/instanceKlass.cpp
hotspot/src/share/vm/oops/instanceKlass.hpp
--- a/hotspot/src/share/vm/classfile/classFileParser.cpp	Thu Jul 20 15:44:51 2017 +0800
+++ b/hotspot/src/share/vm/classfile/classFileParser.cpp	Thu Jul 20 11:01:20 2017 -0400
@@ -5556,10 +5556,7 @@
   if (anon_last_slash == NULL) {  // Unnamed package
     prepend_host_package_name(_host_klass, CHECK);
   } else {
-    if (!InstanceKlass::is_same_class_package(_host_klass->class_loader(),
-                                              _host_klass->name(),
-                                              _host_klass->class_loader(),
-                                              _class_name)) {
+    if (!_host_klass->is_same_class_package(_host_klass->class_loader(), _class_name)) {
       ResourceMark rm(THREAD);
       THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
         err_msg("Host class %s and anonymous class %s are in different packages",
--- a/hotspot/src/share/vm/oops/instanceKlass.cpp	Thu Jul 20 15:44:51 2017 +0800
+++ b/hotspot/src/share/vm/oops/instanceKlass.cpp	Thu Jul 20 11:01:20 2017 -0400
@@ -2344,47 +2344,40 @@
   return false;
 }
 
+// return true if this class and other_class are in the same package. Classloader
+// and classname information is enough to determine a class's package
 bool InstanceKlass::is_same_class_package(oop other_class_loader,
                                           const Symbol* other_class_name) const {
-  oop this_class_loader = class_loader();
-  const Symbol* const this_class_name = name();
-
-  return InstanceKlass::is_same_class_package(this_class_loader,
-                                             this_class_name,
-                                             other_class_loader,
-                                             other_class_name);
-}
-
-// return true if two classes are in the same package, classloader
-// and classname information is enough to determine a class's package
-bool InstanceKlass::is_same_class_package(oop class_loader1, const Symbol* class_name1,
-                                          oop class_loader2, const Symbol* class_name2) {
-  if (class_loader1 != class_loader2) {
+  if (class_loader() != other_class_loader) {
     return false;
-  } else if (class_name1 == class_name2) {
-    return true;
-  } else {
+  }
+  if (name()->fast_compare(other_class_name) == 0) {
+     return true;
+  }
+
+  {
     ResourceMark rm;
 
     bool bad_class_name = false;
-    const char* name1 = ClassLoader::package_from_name((const char*) class_name1->as_C_string(), &bad_class_name);
-    if (bad_class_name) {
-      return false;
-    }
-
-    const char* name2 = ClassLoader::package_from_name((const char*) class_name2->as_C_string(), &bad_class_name);
+    const char* other_pkg =
+      ClassLoader::package_from_name((const char*) other_class_name->as_C_string(), &bad_class_name);
     if (bad_class_name) {
       return false;
     }
-
-    if ((name1 == NULL) || (name2 == NULL)) {
-      // One of the two doesn't have a package.  Only return true
-      // if the other one also doesn't have a package.
-      return name1 == name2;
+    // Check that package_from_name() returns NULL, not "", if there is no package.
+    assert(other_pkg == NULL || strlen(other_pkg) > 0, "package name is empty string");
+
+    const Symbol* const this_package_name =
+      this->package() != NULL ? this->package()->name() : NULL;
+
+    if (this_package_name == NULL || other_pkg == NULL) {
+      // One of the two doesn't have a package.  Only return true if the other
+      // one also doesn't have a package.
+      return (const char*)this_package_name == other_pkg;
     }
 
-    // Check that package is identical
-    return (strcmp(name1, name2) == 0);
+    // Check if package is identical
+    return this_package_name->equals(other_pkg);
   }
 }
 
--- a/hotspot/src/share/vm/oops/instanceKlass.hpp	Thu Jul 20 15:44:51 2017 +0800
+++ b/hotspot/src/share/vm/oops/instanceKlass.hpp	Thu Jul 20 11:01:20 2017 -0400
@@ -455,11 +455,7 @@
   void set_package(PackageEntry* p) { _package_entry = p; }
   void set_package(ClassLoaderData* loader_data, TRAPS);
   bool is_same_class_package(const Klass* class2) const;
-  bool is_same_class_package(oop classloader2, const Symbol* classname2) const;
-  static bool is_same_class_package(oop class_loader1,
-                                    const Symbol* class_name1,
-                                    oop class_loader2,
-                                    const Symbol* class_name2);
+  bool is_same_class_package(oop other_class_loader, const Symbol* other_class_name) const;
 
   // find an enclosing class
   InstanceKlass* compute_enclosing_class(bool* inner_is_member, TRAPS) const;