RFR: 8151539: Remove duplicate AlwaysTrueClosures
Derek White
derek.white at oracle.com
Wed Mar 9 23:46:44 UTC 2016
Looks nice!
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!
- 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.
>
> ------------------------------------------------------------------------------
>
More information about the hotspot-dev
mailing list