RFR (M) 8207359: Make SymbolTable increment_refcount disallow zero

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Thu Jul 19 17:11:04 UTC 2018



On 7/19/18 12:47 PM, Kim Barrett wrote:
>> On Jul 19, 2018, at 8:34 AM, coleen.phillimore at oracle.com wrote:
>>
>> Please review the revision to this change.   Summary:
>>
>> * made decrement_refcount() use CAS loop.
>> * fixed duplicated logic in try_increment_refcount() thanks to Kim
>> * added gtest case for decrement_refcount.
>> * fixed SA code.
>> * added a bunch of comments
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/8207359.02/webrev
>>
>> Retested with hs-tier1-3.
>> Thanks,
>> Coleen
> Looks good.
>
> A few minor nits for which I don't need another webrev.
>
> ------------------------------------------------------------------------------
> test/hotspot/gtest/classfile/test_symbolTable.cpp
>    87   for (int i = 0; i < PERM_REFCOUNT + 100; i++) {
>    88     bigsym->decrement_refcount();
>    89   }
>    90   ASSERT_EQ(bigsym->refcount(), PERM_REFCOUNT) << "should be sticky";
>
> I think one decrement is enough; no need for the loop.
>
> ------------------------------------------------------------------------------
> src/hotspot/os/solaris/dtrace/jhelper.d
>
> The comment added for nameSymbolLength also applies to the other
> symbol lengths.
>
> ------------------------------------------------------------------------------
> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/Symbol.java
>    78   public long getLength() {
>    79     long i = length.getValue(this.addr);
>    80     return (i >> 16) & 0xffff;
>
> I forgot to mention this before, but I think the length field should
> be renamed to lengthAndRefcount.
>
> ------------------------------------------------------------------------------
>

Okay, I'll fix all these.  Thank you for the code review!
Coleen


More information about the hotspot-runtime-dev mailing list