Incorrect comments in fast-path lightweight unlocking code in the interpreter

[Krystal Mok]

Hi David,

Should it be "zero for successful unlock" ?


If the CAS fails, it means the lock has been inflated, so it'll go to the
slow-path, which calls into the runtime. So the exact meaning should be
"zero for successful light weight unlock"

- Kris

On Fri, Mar 30, 2012 at 7:25 PM, David Holmes <david.holmes at oracle.com>wrote:

> Hi Kris,
>
> --- a/src/cpu/x86/vm/interp_masm_**x86_32.cpp Thu Mar 29 13:02:11 2012
> -0700
> +++ b/src/cpu/x86/vm/interp_masm_**x86_32.cpp Fri Mar 30 17:44:51 2012
> +0800
> @@ -782,7 +782,7 @@
>     if (os::is_MP()) lock();
>     cmpxchgptr(header_reg, Address(obj_reg, 0));
>
> - // zero for recursive case
> + // zero for successful CAS
>     jcc(Assembler::zero, done);
>
> The comment is obviously a copy'n'paste typo. But I think the comment
> should indicate what a successful cas implies. Should it be "zero for
> successful unlock" ? If the cas fails what does that mean?
>
> David
>
>
> On 30/03/2012 7:54 PM, Krystal Mok wrote:
>
>> Hi all,
>>
>> It seems like there's a couple of incorrect comments in the x86 version
>> of InterpreterMacroAssembler::**unlock_object().
>> The SPARC version is not affected.
>>
>> Could anyone please review this patch: [1]
>> The diff is against current tip of jdk8/jdk8/hotspot
>>
>> Regards,
>> Kris Mok
>>
>> [1]: https://gist.github.com/**2250418 <https://gist.github.com/2250418>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20120330/54da1276/attachment.html