RFR: 8209189: Make CompiledMethod::do_unloading more concurrent

[Doerr, Martin]

Hi Erik,

we observe crashes on Big Endian PPC64 machines since this change was pushed.

Taking a look at the IsUnloadingUnion which looks incorrect to me.
Note that sizeof(IsUnloadingUnion) = sizeof(IsUnloadingStruct) = 4 on PPC64.
On Big Endian, the 8 bit _value and the _inflated bits are disjoint.

I haven't checked SPARC, yet.

Can you take a look, please?

Thanks and best regards,
Martin


-----Original Message-----
From: hotspot-dev <hotspot-dev-bounces at openjdk.java.net> On Behalf Of Erik Osterlund
Sent: Montag, 5. November 2018 07:20
To: David Holmes <david.holmes at oracle.com>
Cc: hotspot-dev developers <hotspot-dev at openjdk.java.net>
Subject: Re: RFR: 8209189: Make CompiledMethod::do_unloading more concurrent

Hi David,

Yes that lock is now unused. Sorry I forgot to delete it!

/Erik

> On 4 Nov 2018, at 22:38, David Holmes <david.holmes at oracle.com> wrote:
> 
> Hi Erik,
> 
> Just a follow-up on this change. It seems that the CodeCacheUnloadingTask::_lock is now unused - is that correct? (You (accidentally?) made it non-private as part of this change.)
> 
> I was moving that lock as part of the static initalization cleanup, but will just delete it if now unused.
> 
> Thanks,
> David
> 
>> On 20/10/2018 12:22 AM, Erik Österlund wrote:
>> Hi,
>> Today the nmethods are unloaded either serially or in parallel due to GC (and e.g. class unloading). With ZGC concurrent class unloading, a concurrent fashion is required. Rather than inventing yet a third way of unloading nmethods due to class unloading, it would be ideal if there could be one unified way of doing it that makes everyone happy.
>> To solve this problem in a more general way, a new member function on CompiledMethod is added: is_unloading(). It returns whether a CompiledMethod has broken oops. In order to not have to iterate over all the oops every time this question is asked, it caches the result, so it needs to be computed only once per "epoch". Every time a CodeCache unloading is triggered by the GC, a new epoch is started, which forces re-computation of whether the CompiledMethod is_unloading() the first time it is called.
>> So by having is_unloading() be lazily evaluated, it is now possible to build a do_unloading() method on nmethod that simply makes the nmethod unloaded if it is_unloading(), and otherwise cleans the various caches. Cleaning e.g. the inline caches of said nmethod, uses the same is_unloading() method on its targets to figure out if the IC cache should be cleaned or not. Again, the epoched caching mechanism makes sure we don't recompute this value.
>> The new do_unloading() method may be called in both serial, parallel and concurrent contexts. So I removed the parallel variation of this that we had before, that unnecessarily postponed the unloading due to not having computed whether the nmethod should be unloaded or not. Since that is now computed on-demand lazily, there is no longer a need for postponing this, nor to have two phases for parallel nmethod unloading.
>> While there is more work involved in making concurrent nmethod unloading work, this is a good cleanup towards that goal.
>> Bug ID:
>> https://bugs.openjdk.java.net/browse/JDK-8209189
>> Webrev:
>> cr.openjdk.java.net/~eosterlund/8209189/webrev.00/
>> Thanks,
>> /Erik