RFR: 8293251: Use stringStream::base() instead of as_string() when applicable

[Thomas Stuefe]

On Fri, 2 Sep 2022 11:20:59 GMT, Johan Sjölén <duke at openjdk.org> wrote:

> Hi!
> 
> Please review this PR swapping out stringStream::as_string() with ::base() when applicable. With this change we avoid allocating a resource managed string copy. I also attempt to document the base function, as it is not safe to use any result returned from it if you subsequently call the stringStream's methods.
> 
> When I've left ResourceMarks in place I've also commented which calls requires them.
> 
> This passes tier1, tier2 tests.

Hi @jdksjolen,

Generally good, see remarks inline.

Cheers, Thomas

src/hotspot/share/code/codeCache.cpp line 1485:

> 1483:                  get_code_heap_flag_name(code_blob_type));
> 1484:       const char *msg1 = msg1_stream.base();
> 1485:       const char *msg2 = msg2_stream.base();

I would do this only in places where it's super-obvious nothing bad will happen and where it cannot bitrot. Here it's not, since you assign it to a temporary variable. Someone inattentive may modify it in the future, forgetting about why this is special. 

Since the error is hard to catch and the benefit tiny, I would leave this as `as_string`. This is just my gut feeling, others may disagree.

src/hotspot/share/utilities/ostream.hpp line 221:

> 219:   // A stringStream may free the pointer returned
> 220:   // if any of its methods are called, therefore
> 221:   // take care in using this method.

Comment is a bit misleading, what does "borrow" mean? 

Suggestion: "Returns internal buffer. Returned buffer is only guaranteed to be valid as long as stream is not modified"

src/hotspot/share/utilities/vmError.cpp line 576:

> 574:       stringStream message;
> 575:       message.print("This is a message with no ResourceMark");
> 576:       tty->print_cr("%s", message.base());

I think the intent was to use as_string deliberately to get an mark-missing crash.

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

Changes requested by stuefe (Reviewer).

PR: https://git.openjdk.org/jdk/pull/10142