RFR: 8259231: Fix the chance to allocate failure and improve the speed and quality of EpsilonHeap::allocate_work

[Lehua Ding]

On Tue, 5 Jan 2021 13:39:04 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Hi all,
>> 
>> The `EpsilonHeap::allocate_work` method maybe can be fixed and improved by this:
>> 1. it can prevent allocate failure by retry `_space->par_allocate` before expanding virtual space,  when there not enough virtual space but another thread expanding succeeded just and has enough space.
>> 2. it can reduce the lock time by move `res = _space->par_allocate(size);` out of lock scope.
>> 
>> Test on macosx-x86_64-server-{release, fastdebug, slowdebug} with current test case.
>
> src/hotspot/share/gc/epsilon/epsilonHeap.cpp line 113:
> 
>> 111:   while (res == NULL) {
>> 112:     // Allocation failed, attempt expansion, and retry:
>> 113:     {
> 
> I see what you are trying to do, and it makes sense. I believe this form would be cleaner:
> 
> HeapWord* res = NULL;
> while (true) {
>   // Try to allocate, assume space is available
>   res = par_allocate(size);
>   if (res != NULL) {
>     break;
>   }
> 
>   MutexLocker ml(Heap_Lock);
> 
>   // Try to allocate under the lock, assume another thread was able to expand
>   res = par_allocate(size);
>   if (res != NULL) {
>     break;
>   }
> 
>   // Expand and loop back if space is available
>   size_t space_left = max_capacity() - capacity();
>   size_t want_space = MAX2(size, EpsilonMinHeapExpand);
>    ...
> }

Yes, the new form is cleaner very. And I think it would be a little cleaner if wrap the lock scope in curly braces. like this:

++
HeapWord* res = NULL;
while (true) {
  // Try to allocate, assume space is available
  res = par_allocate(size);
  if (res != NULL) {
    break;
  }

  {
    MutexLocker ml(Heap_Lock);
  
    // Try to allocate under the lock, assume another thread was able to expand
    res = par_allocate(size);
    if (res != NULL) {
      break;
    }
  
    // Expand and loop back if space is available
    size_t space_left = max_capacity() - capacity();
    size_t want_space = MAX2(size, EpsilonMinHeapExpand);
     ...
  }
}

-------------

PR: https://git.openjdk.java.net/jdk/pull/1794