> Add a flag _suppress_cr to outputStream. outstream objects won't emit any CR if it's set.
> Correct TypeInstPtr::dump2 to make sure it only emits klass name once. > Remove the comment because Klass::oop_print_on() has emitted the address of oop. > > Before: > 689 ConP === 0 [[ 821 ]] Oop:java/lang/Stringjava.lang.String > {0x000000010159d7c8} - klass: public final synchronized 'java/lang/String' > - string: "a" > :Constant:exact * > > After: > 689 ConP === 0 [[ 821 ]] Oop:java.lang.String {0x000000010159d7c8} - klass: public final synchronized 'java/lang/String' - string: "a":Constant:exact * 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. ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2178/files - new: https://git.openjdk.java.net/jdk/pull/2178/files/077f9b60..6df63fe1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2178&range=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2178&range=05-06 Stats: 83 lines in 5 files changed: 0 ins; 75 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/2178.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2178/head:pull/2178 PR: https://git.openjdk.java.net/jdk/pull/2178 |
On Tue, 23 Feb 2021 23:12:56 GMT, Xin Liu <[hidden email]> wrote:
>> Add a flag _suppress_cr to outputStream. outstream objects won't emit any CR if it's set. >> Correct TypeInstPtr::dump2 to make sure it only emits klass name once. >> Remove the comment because Klass::oop_print_on() has emitted the address of oop. >> >> Before: >> 689 ConP === 0 [[ 821 ]] Oop:java/lang/Stringjava.lang.String >> {0x000000010159d7c8} - klass: public final synchronized 'java/lang/String' >> - string: "a" >> :Constant:exact * >> >> After: >> 689 ConP === 0 [[ 821 ]] Oop:java.lang.String {0x000000010159d7c8} - klass: public final synchronized 'java/lang/String' - string: "a":Constant:exact * > > 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. Changes requested by thartmann (Reviewer). 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`. 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 `)`. ------------- PR: https://git.openjdk.java.net/jdk/pull/2178 |
On Wed, 24 Feb 2021 06:50:51 GMT, Tobias Hartmann <[hidden email]> 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 |
Free forum by Nabble | Edit this page |