RFR: 8220366: Optimize Symbol handling in ClassVerifier and SignatureStream

[Claes Redestad]

On 2019-03-14 16:47, Lois Foltan wrote:
> 
> Hi Claes,
> 
> Looks like a good improvement! 

Thanks!

> A couple of comments:
> 
> - src/hotspot/share/oops/symbol.cpp - new method Symbol::make_permanent()
>    I have a concern that the while (true) loop could be infinite, can 
> you have Coleen ok this part of the change since she is the expert in 
> regards to Symbol refcounts?

I've checked this with Coleen. The race handling logic here mimics that
of increment_ and decrement_refcount. A live lock condition where two or
more threads keep trying to update a refcount indefinitely should be
extremely rare, in practice they should always resolve within a few
iterations.

> 
> - src/hotspot/share/runtime/signature.cpp
>    The removal of SignatureVerifier::invalid_name_char(char c) method 
> seems fine except that the new switch at line #496 does not include the 
> ';' case, why?

Note that the ';' case is handled above the switch. Admittedly, this
expression can be simplified a bit:

diff -r 89b091544fd0 src/hotspot/share/runtime/signature.cpp
--- a/src/hotspot/share/runtime/signature.cpp	Thu Mar 14 16:50:03 2019 +0100
+++ b/src/hotspot/share/runtime/signature.cpp	Thu Mar 14 17:09:35 2019 +0100
@@ -479,10 +479,9 @@
      case 'L':
        for (index = index + 1; index < limit; ++index) {
          char c = type[index];
-        if (c == ';') {
-          return index + 1;
-        }
          switch (c) {
+          case ';':
+            return index + 1;
            case '\0': case '.': case '[':
              return -1;
            default: ; // fall through

Maybe that's clearer? Seeing that the code is ASSERT-only since
JDK-8219579 the changes in this area are mostly cosmetic.

> - copyrights need to be updated in various files

Sure.

/Claes