RFR: 8149837: String.intern creates more work than necessary for G1

[Stefan Karlsson]

Hi Derek,

I don't think this change will work, unless something changed in this 
area recently. We used to have the code the way your patch is written, 
but unfortunately we end up with a lock reordering problem with the 
StringTable_lock and one of the locks taken when executing the  
G1SATBCardTableModRefBS::enqueue. Unfortunately, there's no comment in 
the code that shows this intricate problem.

This is the problematic code:

  241
  242   // Grab the StringTable_lock before getting the_table() because it could
  243   // change at safepoint.
  244   oop added_or_found;
  245   {
  246     MutexLocker ml(StringTable_lock, THREAD);
  247     // Otherwise, add to symbol to table
  248     added_or_found = the_table()->basic_add(index, string, name, len,
  249                                   hashValue, CHECK_NULL);
  250   }
  251
  252   ensure_string_alive(added_or_found);
  253
  254   return added_or_found;
  255 }

We hold StringTable_lock, while calling this chain: basic_add -> 
lookup_in_main_table -> ensure_string_alive -> 
G1SATBCardTableModRefBS::enqueue.

I'm not sure, but I think this was the code that took the lock:
void PtrQueueSet::enqueue_complete_buffer(void** buf, size_t index) {
   MutexLockerEx x(_cbl_mon, Mutex::_no_safepoint_check_flag);

and if that's the case you might be able to provoke the problem by 
setting the G1SATBBufferEnqueueingThresholdPercent flag.

Have you run this patch with fastdebug on a larger set of benchmarks?

I think we should hold this patch until we have figured out if this is 
still a problem or not.

Thanks,
StefanK

On 2016-02-15 20:52, Derek White wrote:
> This small change only does a STAB read barrier if we really read a 
> string from the string intern table.
>
> This is a small optimization, and the old code may contribute to the 
> malloc failures in JDK-8134992 
> <https://bugs.openjdk.java.net/browse/JDK-8134992> 
> "vm/gc/compact/Compact_InternedStrings_Strings failed due to a 
> malloc() failure"
>
> RFE: JDK-8149837 <https://bugs.openjdk.java.net/browse/JDK-8149837>
> Webrev: http://cr.openjdk.java.net/~drwhite/8149837/webrev.01/
>
> Tested: jprt
>
> Thanks!
>  - Derek

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20160215/4c036673/attachment.htm>