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