8006742: Initial TLAB sizing heuristics might provoke premature GCs

[Thomas Schatzl]

Hi Roshan,

On Wed, 2018-06-13 at 16:53 +0530, roshan mangal wrote:
> Hi Everyone,
> 
> This is my first patch as a new member of OpenJDK community.
> 
> I have looked into minor bug
> https://bugs.openjdk.java.net/browse/JDK-8006742 ( Initial TLAB
> sizing
> heuristics might provoke premature GCs )
> 
> Issue: -
> The issue is due to late update of  average threads count
> "global_stats()->allocating_threads_avg".
> The method "global_stats()->allocating_threads_avg()"  always returns
> 1
> until first young GC happens.
> 
> ThreadLocalAllocBuffer::initial_desired_size() returns  "init_sz=
> tlab_capacity/ allocating_threads_avg *target_refills"
> 
>   i.e  init_sz = tlab_capacity/1*50.
> 
> Due to above calculation young GC happens before creating first 50
> threads.
> 
> Issue happens with below command in jdk11 :-
> $java -Xmn3520m -Xms3584m -Xmx3584m -XX:+PrintGC
> -XX:+UseParallelOldGC
> -XX:+UseParallelGC Threads 64
> [0.001s][warning][gc] -XX:+PrintGC is deprecated. Will use -Xlog:gc
> instead.
> [0.004s][info   ][gc] Using Parallel
> [0.209s][info   ][gc] GC(0) Pause Young (Allocation Failure)
> 2640M->1M(3144M) 14.863ms
> 
> Proposed Solution:
> The variable "GlobalTLABStats:: _allocating_threads"  should be
> updated
> with each thread creation.
> So incremented  "GlobalTLABStats:: _allocating_threads" inside
> ThreadLocalAllocBuffer::initialize ( call stack:- Thread ->
> initialize_tlab() ->  tlab().initialize() ) .
> 
> Please find the patch below.
> 
> ======================== PATCH
> ==========================================
> 
> diff -r d12828b7cd64
> src/hotspot/share/gc/shared/threadLocalAllocBuffer.cpp
> --- a/src/hotspot/share/gc/shared/threadLocalAllocBuffer.cpp    Wed
> Jun 13
> 10:15:35 2018 +0200
> +++ b/src/hotspot/share/gc/shared/threadLocalAllocBuffer.cpp    Wed
> Jun 13
> 05:08:01 2018 -0500
> @@ -192,7 +192,8 @@
>    initialize(NULL,                    // start
>               NULL,                    // top
>               NULL);                   // end
> -
> +  global_stats()->update_allocating_threads();
> +  global_stats()->publish();
>    set_desired_size(initial_desired_size());
> 
>    // Following check is needed because at startup the main
> 
> =====================================================================
> ===

  the change does solve the issue, but the problem is that the global
data that is updated and changed here is updated/changed here without
proper synchronization (at least GlobalTLABStats::_allocating_threads)
from what I understand the code.

I.e. Threads::initialize_tlab() as called by the Java thread entry
point JavaThread::run() and also by the attach_current_thread() method
is not synchronized in any way, so this may result in garbage further
along.

Unfortunately I do not have a quick other solution for this - also
because you probably do not want to some heavyweight synchronization
(lock) in that path is it would likely decrease java thread creation
throughput.

Thanks,
  Thomas