RFR(XXS) 8249087 Symbol constructor unnecessarily initializes _body[0]

[Ioi Lam]

On 7/20/20 7:36 PM, David Holmes wrote:
> Hi Ioi,
>
> On 21/07/2020 10:12 am, Ioi Lam wrote:
>> Hi please review this very simple fix:
>>
>> diff -r 19f26d72a8d0 src/hotspot/share/oops/symbol.cpp
>> --- a/src/hotspot/share/oops/symbol.cpp    Mon Jul 20 14:24:19 2020 
>> -0700
>> +++ b/src/hotspot/share/oops/symbol.cpp    Mon Jul 20 17:11:57 2020 
>> -0700
>> @@ -51,8 +51,11 @@
>>   Symbol::Symbol(const u1* name, int length, int refcount) {
>>     _hash_and_refcount = pack_hash_and_refcount((short)os::random(), 
>> refcount);
>>     _length = length;
>> -  _body[0] = 0;  // in case length == 0
>>     memcpy(_body, name, length);
>> +  // For symbols of length 0 and 1: _body[0] (and _body[1]) are 
>> uninitialized and may
>
> Can we ever have a Symbol of length zero?
>
> If the Symbol name is length 1 then surely _body[0] is initialized to 
> the single character of that name?
>
> The change seems harmless given a zero length symbol is meaningless, 
> but the commentary just confuses things to me.
>

Hi David,

We can have a valid Symbol of length 0. All UTF8 constants in 
classfiles, including the literal string "", are represented as Symbols.

How about

// Random, uninitialized values may appear in _body[0] and _body[1]
// for Symbols of length 0 and 1. These random values never change during
// the lifetime of the Symbol, and are read only by Symbol::identity_hash(),
// which would tolerate such randomness.


Thanks
- Ioi
> Thanks,
> David
> -----
>
>> +  // contain random values, which will only be read by 
>> Symbol::identity_hash(),
>> +  // which would tolerate such randomness. These values never change 
>> during the lifetime
>> +  // of the Symbol.
>>   }
>>
>>
>> Passed hs tiers 1/2. Running tiers 3/4 now.
>>
>> Thanks
>> - Ioi