Bug ID: JDK-8057846 ClassVerifier::change_sig_to_verificationType temporary symbol creation code is hot

Versions (Unresolved/Resolved/Fixed)

The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.

JDK 9
9 b35Fixed

If one runs a classloader benchmark:
  http://cr.openjdk.java.net/~shade/8053904/benchmarks.jar

...with Nashorn-generated class files:
 http://cr.openjdk.java.net/~shade/8053904/classes.jar

...and jdk9 executed as:
 $ java -jar benchmarks.jar -wi 0 -i 30 -r 10s -p file=classes.jar

...one can produce this profile:
 http://cr.openjdk.java.net/~shade/8053904/class-install-calltree-2.txt

Looking closely at this profile, one can note an interesting thing: ClassVerifier::change_sig_to_verificationType seems to spend a lot of time creating the temporary symbols:

 ClassVerifier::change_sig_to_verificationType seems to copy the symbols a lot:
  +- 30.740 (9%) ClassVerifier::change_sig_to_verificationType(SignatureStream*,VerificationType*,Thread*)
  | +- 15.070 (4%) SignatureStream::as_symbol(Thread*)
  | +- 14.850 (4%) ClassVerifier::create_temporary_symbol(const Symbol*,int,int,Thread*)

This seems to be an offending piece of code:

inline int ClassVerifier::change_sig_to_verificationType(
    SignatureStream* sig_type, VerificationType* inference_type, TRAPS) {
  BasicType bt = sig_type->type();
  switch (bt) {
    case T_OBJECT:
    case T_ARRAY:
      {
        Symbol* name = sig_type->as_symbol(CHECK_0);
        // Create another symbol to save as signature stream unreferences
        // this symbol.
        Symbol* name_copy =
          create_temporary_symbol(name, 0, name->utf8_length(), CHECK_0);
        assert(name_copy == name, "symbols don't match");
        *inference_type =
          VerificationType::reference_type(name_copy);
        return 1;
      }

...and it comes from a mammoth changeset:

$ hg log -r 2062
changeset:   2062:3582bf76420e
user:        coleenp
date:        Thu Jan 27 16:11:27 2011 -0800
summary:     6990754: Use native memory and reference counting to implement SymbolTable
 
I think we need to reconsider how we manage symbol lifecycle to evade unreferencing without doing a copy.

Awesome!
25-09-2014
Thumbs up! Please push this change out. The bottleneck is gone from Nashorn classloading profiles. Loading the complete generated code from Nashorn/Octane compile (java -jar ~shade/8053904/benchmarks.jar -p file=~shade/8053904/classes.jar) yields the predicted improvement: before: 632 +- 4 ms after: 600 +- 3 ms +5% improvement
25-09-2014
I fortunately remembered why I put that code there in the first place. :)
25-09-2014
Cool. I cowardly bailed out from trying something like that. Let me try how that performs.
25-09-2014
Sure, that's easier and it's a super-simple patch. It was as you suggested: --- a/src/share/vm/classfile/verifier.hpp +++ b/src/share/vm/classfile/verifier.hpp @@ -409,10 +409,17 @@ // their reference counts need to be decrememented when the verifier object // goes out of scope. Since these symbols escape the scope in which they're // created, we can't use a TempNewSymbol. - Symbol* create_temporary_symbol( - const Symbol* s, int begin, int end, TRAPS); + Symbol* create_temporary_symbol(const Symbol* s, int begin, int end, TRAPS); Symbol* create_temporary_symbol(const char s, int length, TRAPS); + Symbol create_temporary_symbol(Symbol* s) { + // This version just updates the reference count and saves the symbol to be + // derefenced later. + s->increment_refcount(); + _symbols->push(s); + return s; + } + TypeOrigin ref_ctx(const char* str, TRAPS); }; @@ -425,10 +432,8 @@ case T_ARRAY: { Symbol* name = sig_type->as_symbol(CHECK_0); - // Create another symbol to save as signature stream unreferences - // this symbol. - Symbol* name_copy = - create_temporary_symbol(name, 0, name->utf8_length(), CHECK_0); + // Create another symbol to save as signature stream unreferences this symbol. + Symbol* name_copy = create_temporary_symbol(name); assert(name_copy == name, "symbols don't match"); *inference_type = VerificationType::reference_type(name_copy);
25-09-2014
Sure, why not?
22-09-2014
Guys, I need (or really badly want, rather) this in 9, or even 8u40-8u60 if possible. 10 seems a bit... futuristic. Regards Marcus
21-09-2014
Why this is targeted for 10, not for 9?
17-09-2014