RFR (M): (7u60): Backport of 8027476: Improve performance of Stringtable unlink, 8027455: Improve symbol table scan times during gc pauses

[Thomas Schatzl]

Hi,

  I got the link for the new webrev wrong, taking two from the original
patch. Sorry.

On Tue, 2014-01-28 at 10:51 +0100, Thomas Schatzl wrote:
> Hi all,
> 
>   could I have reviews for the following backport of the symbol table/
> string table parallelization change?
> 
> It implements parallelization of string table and symbol table scan
> during G1 full GC and remark for 7u60.
> 
> There is some difference in the patches due to how 7u60 managing the
> iteration over the string table:
> 
> - 7u60 has no StringTable/StringTable::unlink_or_oops_do() methods,
> but only separate StringTable/SymbolTable::unlink() and
> StringTable/SymbolTable::oops_do() methods.
> 
> - 7u60 allows sharing of strings using CDS, making minor changes
> necessary.

In particular, in StringTable::buckets_unlink(), the code that considers
shared strings need to be kept alive, i.e. in
http://cr.openjdk.java.net/~tschatzl/8027476.7u60/webrev/src/share/vm/classfile/symbolTable.cpp.frames.html , in the right (new) version the following code.

 820       // Shared entries are normally at the end of the bucket and
if we run into
 821       // a shared entry, then there is nothing more to remove. However, if we
 822       // have rehashed the table, then the shared entries are no longer at the
 823       // end of the bucket.
 824       if (entry->is_shared() && !use_alternate_hashcode()) {
 825         break;
 826       }
 827       assert(entry->literal() != NULL, "just checking");
 828       if (entry->is_shared() ||
is_alive->do_object_b(entry->literal())) {

In 8u20/9 the corresponding code is just:

http://cr.openjdk.java.net/~tschatzl/8027476/webrev.1/src/share/vm/classfile/symbolTable.cpp.frames.html , again, right frame:

 835       assert(!entry->is_shared(), "CDS not used for the
StringTable");
 836 
 837       if (is_alive->do_object_b(entry->literal())) {

Automatic merge got confused and simply did not take this difference
into account. I hope this clears this recent, rather cryptic comment up.

> These differences are contained in symbolTable.?pp, the rest is
> verbatim backport iirc.
> 
> New webrev:
> http://cr.openjdk.java.net/~tschatzl/8027476/webrev

Correct webrev is at:

http://cr.openjdk.java.net/~tschatzl/8027476.7u60/webrev

> Old Webrev:
> http://cr.openjdk.java.net/~tschatzl/8027476/webrev.1
> 
> CRs:
> https://bugs.openjdk.java.net/browse/JDK-8027455
> https://bugs.openjdk.java.net/browse/JDK-8027476
> 
> Testing:
> jtreg test case, jprt, FMW apps
> 
> Thanks,
>   Thomas

Sorry,
  Thomas