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

Thomas Stuefe stuefe at openjdk.org
Wed Sep 14 12:57:37 UTC 2022


On Wed, 14 Sep 2022 12:03:52 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.
>
> Johan Sjölén has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Missed fixes

Hi Johan,

please be aware that changes like these across the code base makes downporting more painful. So I would only do wholesale renames and replacements where the benefit clearly outweighs the downporting costs.

About your last iteration, yes, I think requiring base() to freeze the stream is a bit much.

How about renaming "internal_string" to "freeze"


// Freezes stringStream (no further modifications possible) and returns pointer to it.
// No-op if stream is frozen already.
const char* freeze();


"freeze" is snappy and clear in its meaning. And in release builds I also would freeze, allowing no further changes, apart from the debug-only assert. Since writes potentially reallocate the buffer, the char ptr may be invalid, better to have incomplete stringStream output than C-heap corruption (as improbable as it is).

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

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


More information about the hotspot-dev mailing list