RFR: 8151539: Remove duplicate AlwaysTrueClosures

[Mikael Gerdin]

Stefan,

On 2016-03-15 08:39, Stefan Karlsson wrote:
> Hi reviewers,
>
> Are you OK with the latest patch?

Looks good to me.

/Mikael

>
> Thanks,
> StefanK
>
> On 10/03/16 09:58, Stefan Karlsson wrote:
>> Hi Kim,
>>
>> Updated webrevs:
>>  http://cr.openjdk.java.net/~stefank/8151539/webrev.02.delta/
>>  http://cr.openjdk.java.net/~stefank/8151539/webrev.02/
>>
>> I moved the AlwaysTrueClosure to iterator.hpp, and created a stack
>> local instance in jniHandles.cpp.
>>
>> I also found that there's an AlwaysFalseClosure, so I took the liberty
>> of moving this to iterator.hpp as well.
>>
>> Just a few comments inlined:
>>
>> On 2016-03-10 00:11, 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.
>>
>> Thanks for pointing that out. It's not a problem here though, but I'll
>> clean this out to prevent future bugs our questions.
>>
>>>
>>> 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.)
>>
>> I agree. I've changed it to a stack instance.
>>
>>>
>>> ------------------------------------------------------------------------------
>>>
>>> src/share/vm/gc/shared/referenceProcessor.cpp
>>>   269   class AlwaysAliveClosure: public BoolObjectClosure {
>>>
>>> This local class can go away too.
>>
>> Done.
>>
>> Thanks,
>> StefanK
>>
>>>
>>> ------------------------------------------------------------------------------
>>>
>>> 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.
>>>
>>> ------------------------------------------------------------------------------
>>>
>>>
>>
>