Request for review (S): 4988100: oop_verify_old_oop appears to be dead

[Jesper Wilhelmsson]

Bengt,

I looked at the updated webrev and it looks good.
Ship it!
/Jesper


On 2012-04-16 11:07, Bengt Rutisson wrote:
>
> Hi Alex,
>
> On 2012-04-15 20:47, Alex Lam S.L. wrote:
>> Hi there,
>>
>> My (java.net - is that the one?) username is "alexlamsl".
>
> No, this is not the user name I was looking for. The user names that can be
> used in OpenJDK changesets are OpenJDK user names. These are listed here:
> http://db.openjdk.java.net/people
>
> To get an OpenJDK user name you need to become an author in one of the OpenJDK
> projects. See the section "Becoming an Author" here:
> http://openjdk.java.net/projects/
>
> You first need to become a contributor:
> http://openjdk.java.net/bylaws#contributor
>
> And then send an email request to become an author to the project lead. Here
> it says that this is Paul Hohensee:
> http://openjdk.java.net/census#hsx
>
> But I think it should be John Coomes. Let's sort this out offline if you would
> like to become an OpenJDK author.
>
>> Sorry for not spotting this earlier:
>>
>> src/share/vm/oops/instanceRefKlass.cpp
>>
>> line 505, 508, 523& 526
>>
>> I don't know if the call to "java_lang_ref_Reference::next_addr" is
>> side-effect-free, but at least we can remove the "if
>> (UseCompressedOops)" blocks since they are now identical in both
>> cases?
>
> Good point. There are no side effects by these calls so it should be safe to
> remove them.
>
> Here's an updated wevrev:
> http://cr.openjdk.java.net/~brutisso/4988100/webrev.02/
>
> Thanks again for looking at this!
> Bengt
>
>>
>> Regards,
>> Alex.
>>
>>
>>
>> On Fri, Apr 13, 2012 at 9:48 PM, Bengt Rutisson
>> <bengt.rutisson at oracle.com> wrote:
>>> Hi Alex,
>>>
>>>
>>> On 2012-04-13 20:09, Alex Lam S.L. wrote:
>>>> src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
>>>>
>>>> can _allow_dirty in G1ParVerifyTask be removed?
>>>
>>>
>>> Good catch! I removed it. Here is an updated webrev:
>>> http://cr.openjdk.java.net/~brutisso/4988100/webrev.01/
>>>
>>> BTW, you don't seem to have an OpenJDK user name. I listed you as a reviewer
>>> in the summary using your email address.
>>>
>>> Thanks again for looking at this!
>>> Bengt
>>>
>>>
>>>>
>>>> Alex.
>>>>
>>>>
>>>>
>>>> On Fri, Apr 13, 2012 at 5:53 PM, Bengt Rutisson
>>>> <bengt.rutisson at oracle.com> wrote:
>>>>> Jon,
>>>>>
>>>>> Thanks for the review! Anybody else feel like taking a look? You all
>>>>> heard
>>>>> Jon: with the Next link it is easy to review this :-)
>>>>>
>>>>> Bengt
>>>>>
>>>>>
>>>>> On 2012-04-13 18:31, Jon Masamitsu wrote:
>>>>>> Bengt,
>>>>>>
>>>>>> This looks good and it's a nice clean up. And thanks for redoing the
>>>>>> webrev. Having the "Next" link on each page made it even more
>>>>>> convenient than I expected.
>>>>>>
>>>>>> Jon
>>>>>>
>>>>>> On 4/10/2012 6:13 PM, Bengt Rutisson wrote:
>>>>>>>
>>>>>>> Hi all,
>>>>>>>
>>>>>>> Could I have a couple of reviews for this small change (a lot of files,
>>>>>>> but very simple):
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~brutisso/4988100/webrev.00/
>>>>>>>
>>>>>>> While going through old CRs we found this one that stated that
>>>>>>> oop_verify_old_oop() was unused as well as the allow_dirty parameter
>>>>>>> that is
>>>>>>> being passed around. I'd like to fix it to be able to close the CR.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Bengt
>>>>>
>