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

[Mikael Gerdin]

On Friday 10 January 2014 17.24.33 Mikael Gerdin wrote:
> Thomas,
> 
> Overall I think the change is fine.
> I have some comments inline,
> 
> On Friday 10 January 2014 10.27.49 Thomas Schatzl wrote:
> > Hi all,
> > 
> >   could I have reviews for the following performance change?
> > 
> > It implements parallelization of string table and symbol table scan
> > during G1 full GC and remark.
> > 
> > This decreases the respective string table/symbol table scan times
> > almost linearly depending on the number of threads used.
> > 
> > The change is quite straightforward, similar to string table scan during
> > initial mark there are new possibly_parallel_unlink() methods for both
> > string and symbol table, that are later used in a new taskset.
> > 
> > Task distribution works as before, using a single claim value for each
> > of the tables (reusing the one for the string table) on the hash table
> > array.
> > 
> > The impact on total remark/full gc time is significant, in conjunction
> > with JDK-8027454 (RFR coming soon) remark time is halved.
> > 
> > I decided to merge these two fixes into a single webrev because these
> > two things are closely related, and the shared code between the two
> > issues is very large, and the actual code to implement one or the other
> > is small and easily understood (imo). Splitting would add lots of code
> > that would first need to be added and then removed again too.
> > 
> > I filed two follow-up CRs to use these new methods in the other parallel
> > collectors too (JDK-8031473), and another one for fine-tuning
> > (JDK-8031472) some parameters.
> > 
> > Webrev:
> > http://cr.openjdk.java.net/~tschatzl/8027476/webrev
> 
> -  // Now clean up stale oops in StringTable
> -  StringTable::unlink(&g1_is_alive);
> -  // Clean up unreferenced symbols in symbol table.
> -  SymbolTable::unlink();
> +  g1h->unlink_string_and_symbol_table(&g1_is_alive);
> 
> Why did we call StringTable::unlink here?
> AFAICT G1's initial mark calls process_strong_roots with SO_Strings with the
> G1ParScanAndMarkExtRootClosure, which in turn will make all strings which
> are in the StringTable at init mark to be live. Have you ever seen any
> strings being unloaded at this point? ( See
> https://bugs.openjdk.java.net/browse/JDK-8015332 and
> http://openjdk.java.net/jeps/156 )

Nevermind, I just read 8027454...
/Mikael

> 
> 
> +  ~G1StringSymbolTableUnlinkTask() {
> +    guarantee(!_process_strings || StringTable::parallel_claimed_index() >=
> _initial_string_table_size,
> +              err_msg("claim value "SIZE_FORMAT" after unlink less than
> initial string table size "SIZE_FORMAT,
> +                      StringTable::parallel_claimed_index(),
> _initial_string_table_size));
> +  }
> 
> Why do you only check the string table size here and not the symbol table?
> 
> 
> /Mikael
> 
> > CRs:
> > https://bugs.openjdk.java.net/browse/JDK-8027455
> > https://bugs.openjdk.java.net/browse/JDK-8027476
> > 
> > Testing:
> > test case, jtreg, FMW apps, specjbb2005, specjbb2013, dacapo, and lots
> > of other testing in the recent months
> > 
> > Thanks,
> > 
> >   Thomas