RFR (L) JDK-8195100: Use a low latency hashtable for SymbolTable
Ioi Lam
ioi.lam at oracle.com
Thu Aug 2 16:26:47 UTC 2018
On 8/2/18 8:44 AM, Gerard Ziemski wrote:
>> On Aug 2, 2018, at 10:40 AM, Ioi Lam <ioi.lam at oracle.com> wrote:
>>
>>
>>
>> On 8/2/18 8:26 AM, Gerard Ziemski wrote:
>>>> symbolTable.cpp:
>>>>
>>>> 342 // Otherwise, add the symbol to table. Copy to a C string first.
>>>> 343 char stack_buf[ON_STACK_BUFFER_LENGTH];
>>>> 344 ResourceMark rm(THREAD);
>>>> 345 if (len <= ON_STACK_BUFFER_LENGTH) {
>>>> 346 buffer = stack_buf;
>>>> 347 } else {
>>>> 348 buffer = NEW_RESOURCE_ARRAY_IN_THREAD(THREAD, char, len);
>>>> 349 }
>>>> 350 for (int i=0; i<len; i++) {
>>>> 351 buffer[i] = name[i];
>>>> 352 }
>>>> 353 // Make sure there is no safepoint in the code above since name can't move.
>>>> 354 // We can't include the code in NoSafepointVerifier because of the
>>>> 355 // ResourceMark.
>>>>
>>>> The above is dead code and should be removed.
>>> That’s not dead code, but in fact you did find an issue here - we should be using the “buffer”, not “name” when we add to the table. Thanks!
>>>
>>>
>> Why do you need to make a copy here? buffer and name seems to have the exact same contents.
> I didn’t write that code, but I assume the name comes in some temp memory that can go away at any time, so we need a solid copy.
>
>
> cheers
>
This function is called only from two places. In both cases, the input
Symbol won't go away
Symbol* SignatureStream::as_symbol(TRAPS) {
// Create a symbol from for string _begin _end
int begin = _begin;
int end = _end;
if ( _signature->byte_at(_begin) == 'L'
&& _signature->byte_at(_end-1) == ';') {
begin++;
end--;
}
// Save names for cleaning up reference count at the end of
// SignatureStream scope.
Symbol* name = SymbolTable::new_symbol(_signature, begin, end,
CHECK_NULL);
_names->push(name); // save new symbol for decrementing later
return name;
}
Symbol* ClassVerifier::create_temporary_symbol(const Symbol *s, int begin,
int end, TRAPS) {
Symbol* sym = SymbolTable::new_symbol(s, begin, end, CHECK_NULL);
_symbols->push(sym);
return sym;
}
Also, the "name" was used just a few lines above in the lookup, so if
this was ever a issue, that lookup would fail as well.
Mercurial history shows that this function has not had any meaningful
change since it was imported from SCCS in 2007, so I think it's just a
case of bit rot and the dead code can be safely removed.
Thanks
- Ioi
More information about the hotspot-runtime-dev
mailing list