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