RFR: 8260198: TypeInstPtr::dump2() emits multiple lines if Verbose is set [v7]

Xin Liu xliu at openjdk.java.net
Wed Feb 24 09:50:06 UTC 2021


On Wed, 24 Feb 2021 06:50:51 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> Xin Liu has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8260198: TypeInstPtr::dump2() emits multiple lines if Verbose is set
>>   
>>   use the existing api StringUtils::replace_no_expand to archive the same replace.
>>   don't need to invoke os::strdup because stringStream::as_string() has duplicated
>>   the internal buffer.
>
> src/hotspot/share/opto/type.cpp line 4052:
> 
>> 4050: 
>> 4051:       {
>> 4052:         ResourceMark rm;
> 
> Shouldn't the `ResourceMark` go to before `stringStream ss` which is a `ResourceObj` as well? Also, please add a small comment explaining that this code suppresses the new line emitted by `print_oop`.

hi, @TobiHartmann 
Thank you for reviewing this PR. 

stringStream allocates its dynamic buffer using `NEW_C_HEAP_ARRAY`.  IMHO, it's okay without a ResourceMark. 
Unlike `stringStream::grow`, stringStream::as_string(false) does use `NEW_RESOURCE_ARRAY`, which allocates an array on current thread's resource_area. That's why I put ResourceMark in a syntax scope. 

Actually, the current code still works even without that ResourceMark. It's because `Type::dump_on()` has  declared a rm. Let me hoist ResourceMark as you said. That makes code straight-forward locally and I shouldn't assume its context.

> src/hotspot/share/opto/type.cpp line 4040:
> 
>> 4038: // Dump oop Type
>> 4039: #ifndef PRODUCT
>> 4040: void TypeInstPtr::dump2( Dict &d, uint depth, outputStream* st ) const {
> 
> While you are at it, please also remove the excess whitespace after `(` and before `)`.

done.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2178


More information about the hotspot-compiler-dev mailing list