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