RFR: 8151539: Remove duplicate AlwaysTrueClosures

[Stefan Karlsson]

Thanks, Mikael.

StefanK

On 15/03/16 10:07, Mikael Gerdin wrote:
> 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.
>>>>
>>>> ------------------------------------------------------------------------------ 
>>>>
>>>>
>>>>
>>>
>>