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

[Johan Sjölén]

On Fri, 2 Sep 2022 13:31:46 GMT, Thomas Stuefe <stuefe 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.
>
> 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.

I think it's a very good idea.

> 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"

That's much clearer, thank you for the suggestion.

> 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.

Thank you!

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

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