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

[Bengt Rutisson]

Jon, Jesper and Alex,

Thanks for the reviews! All set to push this now.

Bengt

On 2012-04-16 11:43, Jesper Wilhelmsson wrote:
> 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
>>>>>>
>>