RFR: 8209437: Mechanism for decoupling consumers and providers of behaviours

Wed Aug 22 10:20:50 UTC 2018

Hi Roman,

The problem I'm trying to solve regarding nmethod unloading is to be 
able to call a new function, nmethod::is_unloading(), without having to 
pass around an is_alive closure.

It will be called from the following contexts:
* nmethod::do_unloading() from serial contexts (VMThread), to see if the 
nmethod should be make_unloaded()
* CompiledMethod::cleanup_inline_caches from serial contexts (VMThread), 
to see if IC caches pointing to is_unloading() nmethods should be cleaned
* nmethod::do_unloading() from parallel contexts (STW GC workers), to 
see if the nmethod should be make_unloaded()
* CompiledMethod::cleanup_inline_caches from parallel contexts (STW GC 
workers), to see if IC caches pointing to is_unloading() nmethods should 
be cleaned
* nmethod::do_unloading() from concurrent contexts (concurrent GC 
workers), to see if the nmethod should be make_unloaded()
* CompiledMethod::cleanup_inline_caches from concurrent contexts, to see 
if IC caches pointing to is_unloading() nmethods should be cleaned. 
These contexts comprise:
   - JavaThreads using nmethod entry barriers that will be invoked when 
waking up from a safepoint
   - The sweeper thread running concurrently with class unloading
  * JavaThreads that get IC cache misses into is_unloading() nmethods, 
and deoptimize
  * A whole bunch of random places where we today check if nmethods are 
is_alive(), where we now also need to know that it is not is_unloading()

Passing around is_alive closures from where they are provided through 
these paths is rather messy, while providing an is_alive behaviour from 
serial, parallel, and concurrent contexts, is trivial. With behaviours, 
I only need to provide a PhantomIsAliveBehaviour from 3 places:
1) CodeCache::do_unloading(); serial
2) G1CodeCacheUnloadingTask; parallel
3) A global behaviour; concurrent

...and then I can call is_unloadig() in all these contexts without 
plumbing the call hierarchies with is_alive closures that fill no local 
purpose other than passing it on to the next function to make it 
available in the next scope to eventually check if an nmethod 
is_unloading(), and then I can use the same nmethod unloading code for 
all GCs instead of inventing yet another separate nmethod unloading 
mechanism for the concurrent context.

Thanks,
/Erik

On 2018-08-22 11:51, Roman Kennke wrote:
> Maybe it would be good to start from the beginning. What problem are you
> trying to solve and how to you propose to solve it. I don't yet have a
> clear picture how everything fits together, but so far it smells a
> little like an anti-pattern.
>
> Thank you,
> Roman
>
>> Hi David,
>>
>> I don't know if this pattern does have a name. The closest resembling
>> pattern is a chain of responsibility. But this is more like a layered or
>> stacked chain of responsibility, with thread-local endpoints. It is a
>> pattern I have used in many projects of my own, that I have always found
>> useful.
>>
>> I will try to give a few examples.
>>
>> 1) Unloading nmethods in serial, parallel and concurrent contexts.
>>
>> This is for me the most pressing concrete use-case right now. Today we
>> have one mechanism for serial nmethod unloading triggered by GC. We have
>> another mechanism for parallel nmethod unloading, and now I'm building
>> yet another one for concurrent unloading. What I want to do here is to
>> rather than reinventing the wheel 3 times, find one nmethod unloading
>> mechanism that can be called from serial, parallel and concurrent contexts.
>>
>> A preview (not necessarily final) of how this can be done with
>> behaviours for unifying the serial and parallel cases can be found here:
>> http://cr.openjdk.java.net/~eosterlund/concurrent_class_unloading/webrev.01_03_concurrent_do_unloading/
>>
>>
>> Subsequently, the concurrent nmethod unloading mechanism will re-use the
>> same nmethod cleaning code when we are soon introducing concurrent class
>> unloading. A preview with said behaviour used (again, very much a work
>> in progress) can be found here:
>> http://cr.openjdk.java.net/~eosterlund/concurrent_class_unloading/webrev.01_05_class_unloading/
>>
>>
>> In particular, I provide a new function on nmethod called is_unloading()
>> that basically lazily computes if the nmethod has broken oops in it.
>> Whoever calls it first computes if it is unloading, and subsequent calls
>> use the cached value of what that computation was.
>>
>> In order to compute is_unloading() lazily, a BoolObjectClosure must be
>> available answering the question "is this oop phantomly alive?". And I
>> want this code to not care about howsoever the GC provides the answer in
>> any given context.
>>
>> a) In the serial context, the CodeCache::do_unloading() call will
>> locally provide the behaviour using its passed in OopClosure.
>> b) In the parallel context, the VM thread sets up a local behaviour for
>> this based on the is_alive closure used in the parallel cleanup task.
>> The worker  threads can call do_unloading() on nmethods, because they
>> inherit the context of the VM thread's local scope.
>> c) In the concurrent context, the IC cache cleaning code will also be
>> called by the sweeper and JavaThreads running concurrent to unloading.
>> In this case, ZGC will provide a global phantom is_alive behaviour that
>> can be called from all these contexts.
>>
>> So as you can see, the goal is to make the is_unloading() function that
>> will be called in quite a few contexts not have to care about *how* the
>> phantom is_alive behaviour is available in all contexts it is called
>> from, similar in a way, to how we can call
>> NativeAccess<ON_PHANTOM_OOP_REF> in many contexts without having to know
>> how that behaviour is provided by the GC in that context.
>>
>> 2) Untangling shared GC code
>>
>> Different GCs have slightly different ways of tracking things like time
>> and statistics due to different requirements. This sometimes makes it a
>> nightmare to write code to be shared between different GCs, because
>> anything called that wants to track some time or something, needs to be
>> provided with GC-specific context information, using abstractions that
>> are not shared. With this approach, it is much easier to decouple the
>> callsite that tracks sime time or statistics, to the provider of context
>> information required for it. In particular, we are considering using
>> this for the ZStatCounter.
>>
>> I hope these concrete usages makes it seem more attractive. Again, I do
>> not intend for this to be the silver bullet solution to be abused in all
>> code with variation points.
>>
>> Thanks,
>> /Erik
>>
>> On 2018-08-20 08:55, David Holmes wrote:
>>> Hi Erik,
>>>
>>> Sorry but I find this so abstract I can't quite determine what it is
>>> exactly that you are doing. I suspect I know this pattern under some
>>> other name/terminology but so far, just looking at the code, I can't
>>> connect the dots. Could you give some concrete examples of usage please.
>>>
>>> Thanks,
>>> David
>>>
>>> On 14/08/2018 3:05 AM, Erik Österlund wrote:
>>>> Hi,
>>>>
>>>> Sometimes we find ourselves passing around context information about
>>>> how to perform a certain operation (e.g. check if an oop is alive,
>>>> logging or tracing, etc). Sometimes such behaviours are provided
>>>> globally by a GC, and sometimes only in local scopes known by the GC.
>>>> Sometimes it is even accessed from mutators.
>>>>
>>>> It would be great to have a general mechanism for decoupling how
>>>> behaviours are provided, from the code that uses them.
>>>>
>>>> In particular, I will need this mechanism to build a new nmethod
>>>> unloading mechanism for concurrent class unloading. Today we have a
>>>> single threaded and a parallel nmethod unloading mechanism. Rather
>>>> than introducing a third concurrent way of doing this, I would like
>>>> to unify these mechanism into one mechanism that can be used in all
>>>> three contexts. In order to get there, I need these utilities in
>>>> order to not make a mess. I have a bunch of other use cases down the
>>>> road as well.
>>>>
>>>> The ideas behind this mechanism are pretty straight forward.
>>>> Behaviours are provided in different ways by "behaviour providers".
>>>> The providers may be global and local, but come in a strict layering
>>>> (each provider has a parent). So from a given callsite, there is a
>>>> chain of responsibility with behaviour providers. You can use
>>>> BehaviourMark to provide a behaviour locally in a certain scope.
>>>> There are also global behaviours to which a GC at bootstrapping time
>>>> can add behaviours. If no local behaviour was found, the global
>>>> behaviours are checked as plan B. In order to speed up the walk, the
>>>> scoped behaviour providers also come with a lazily populated
>>>> behaviour provider cache that will take you straight to a given
>>>> provider, effectively skipping through the search through the chain
>>>> of responsibility.
>>>>
>>>> Bug:
>>>> https://bugs.openjdk.java.net/browse/JDK-8209437
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~eosterlund/8209437/webrev.00/
>>>>
>>>> Thanks,
>>>> /Erik