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

Wed Aug 22 10:54:09 UTC 2018

Aha! Now this looks much more useful :-) I've actually run into pretty
much the same problem before and would have welcome this mechanism. I'd
probably have stitched together something via GCThreadLocalData, but a
more explicit and sane way to achieve this would be better.

I'll study the patch some more later (I'm on vacation...)

Thanks, Roman

> 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
> 