RFR: 8151539: Remove duplicate AlwaysTrueClosures

[Stefan Karlsson]

Hi Derek,

On 2016-03-10 00:46, Derek White wrote:
> Looks nice!

Thanks.

>
> Are there ever enough weak JNI handles that it would pay off to 
> re-implement  JNIHandles::weak_oops_do(OopClosure* f) so it doesn't 
> even do the callback? I'd guess not, but thought I'd ask!

It might be worth optimizing this for some workloads, especially since 
this is called from single-threaded code at the moment. If it's really 
important, then we might also want to devirtualize the do_oop calls 
inside JNIHandles::weak_oops_do.

I think we would need a benchmark that shows that this is a problem, 
before we start to optimize this.

Thanks,
StefanK

>
>  - Derek
>
> On 3/9/16 6:11 PM, Kim Barrett wrote:
>>> On Mar 9, 2016, at 3:01 PM, Stefan Karlsson 
>>> <stefan.karlsson at oracle.com> wrote:
>>>
>>> Hi Mikael,
>>>
>>> On 2016-03-09 20:39, Mikael Gerdin wrote:
>>>> Hi Stefan,
>>>>
>>>> On 2016-03-09 17:44, Stefan Karlsson wrote:
>>>>> Hi all,
>>>>>
>>>>> Please review this patch to remove a bunch of redundant
>>>>> AlwaysTrueClosure classes.
>>>>>
>>>>> http://cr.openjdk.java.net/~stefank/8151539/webrev.00/
>>>> There is still
>>>> 2128 // This should be moved to the shared markSweep code!
>>>> 2129 class PSAlwaysTrueClosure: public BoolObjectClosure {
>>>> 2130 public:
>>>> 2131   bool do_object_b(oop p) { return true; }
>>>> 2132 };
>>>> 2133 static PSAlwaysTrueClosure always_true;
>>>> in psParallelCompact.cpp
>>>>
>>>> Otherwise the change looks good, I don't need to see an updated 
>>>> webrev.
>>> Great that you saw this!
>>>
>>> I've uploaded a new webrev, to make it easier for the second reviewer:
>>> http://cr.openjdk.java.net/~stefank/8151539/webrev.01/
>> A couple of things below; otherwise looks good.  I don't need a new
>> webrev for these.
>>
>> ------------------------------------------------------------------------------ 
>>
>> src/share/vm/runtime/jniHandles.cpp
>>   136   static AlwaysTrueClosure always_true;
>>
>> Function-scoped static initialization is not thread-safe in C++03.
>>
>> I don't think there's any noticable benefit to a static here even with
>> thread-safe initialization, as the construction is probably pretty
>> simple. (It might be simple enough that there's no actual
>> thread-safety issue, but why take that chance.)
>>
>> ------------------------------------------------------------------------------ 
>>
>> src/share/vm/gc/shared/referenceProcessor.cpp
>>   269   class AlwaysAliveClosure: public BoolObjectClosure {
>>
>> This local class can go away too.
>>
>> ------------------------------------------------------------------------------ 
>>
>> src/share/vm/gc/shared/genCollectedHeap.cpp
>> [removed]
>>   688 class AlwaysTrueClosure: public BoolObjectClosure {
>>
>> src/share/vm/gc/parallel/psMarkSweep.cpp
>> [removed]
>>   574 class PSAlwaysTrueClosure: public BoolObjectClosure {
>>
>> src/share/vm/gc/parallel/psParallelCompact.cpp
>> [removed]
>> 2129 class PSAlwaysTrueClosure: public BoolObjectClosure {
>>
>> Nice!  Fortunately, these aren't technically ODR violations, since the
>> same-named definitions are identical.
>>
>> ------------------------------------------------------------------------------ 
>>
>>
>