RFR: JDK-8260030: Improve stringStream buffer handling

[Thomas Stuefe]

On Thu, 21 Jan 2021 14:27:29 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> stringStream objects are used as temporary string buffers a lot in hotspot. When investigating JDK-8259710, I found that a large majority of the stringStreams created never use the backing buffer fully; about 70% of all streams write less than 32 characters.
>> 
>> stringStream creates an backing buffer, C-heap allocated, with a default size of 256 characters. Some things could be improved:
>> 
>> - add a small in-object buffer for the first n characters to be written. This would avoid or at least delay allocation of the backing buffer from C-heap, at the expense of slightly increased object size and one memcpy when switching over to C-heap. Note that if the backing buffer is smaller than what now is the default size, the total footprint will go down despite the increased object size.
>> 
>> - Delaying allocation of the backing buffer would be good for the many cases where stringStream objects are created as temporary objects without being actually used, see eg. compile.hpp `class PrintInliningBuffer`
>> 
>> - Besides all that, the code could benefit from a little grooming (using modern style syntax etc).
>> 
>> ----
>> 
>> This patch:
>> 
>> - renames members of stringStream to conform to common syntax (eg leading underscores) and to be clearer
>> - Uses initialization lists
>> - Factors out buffer growth code from stringStream::write() to a new function stringStream::grow()
>> - Introduces a new object-internal mini buffer, `stringStream::_small_buffer`, sized 32 bytes, which is initially used for all writes
>> 
>> This patch drastically reduces the number of malloc calls done from this class. The internal buffer size of 32byte seems a good cut-off. Running some unrelated test program (no tracing active), I see a reduction in the number of malloc calls from stringStream from ~211K malloc calls down to 53K (debug VM). In a release VM, it drops from ~85K down to about 1K. The reason is that `stringStream` is used in a ton of places as a temporary stack allocated object, to write very minimal text snippets to.
>> 
>> I also tweaked the associated gtest to test more thoroughly.
>
> src/hotspot/share/utilities/ostream.hpp line 200:
> 
>> 198:   size_t _capacity;
>> 199:   const bool _is_fixed;
>> 200:   char   _small_buffer[32];
> 
> I'm a little surprised that 32 bytes is that effective. Any idea how much better
> (in malloc avoidance) a few more words would be?  Since 32 bytes isn't really
> all that much.

I was surprised too. I did not want to blow up the stringStream object too much because I was afraid someone may use it on stack in a recursion. I even considered melting the buffer and the buffer pointer into a union to save space, since you only need either one or the other, but the coding got too complex and it felt overengineered.

I did some tests:

java -version

debug:    <32:1038 (86%), <48:111 (9%), <64:42 (3%), >=64:4 (0%)
release:  <32:449 (84%),  <48:54 (10%), <64:26 (4%), >=64:4 (0%)

Running some unrelated test program, no tracing, doing metaspace stuff

debug: 	  <32:151985 (52%), <48:83341 (28%), <64:46916 (16%), >=64:6445 (2%)                  
release:  <32:84123 (49%),  <48:80332 (47%), <64:871 (0%), >=64:3567 (2%) 

same test with -XX:+PrintOptoInlining, which creates many temporary stringStream objects with large buffers

debug:	  <32:160311 (52%), <48:83661 (27%), <64:47821 (15%), >=64:15345 (4%)   

So, depending on the scenario, increasing the buffer size to 48 may give us between 10% and almost 30% more cases. Beyond that the effect strongly diminishes. I will increase it to 48.

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

PR: https://git.openjdk.java.net/jdk/pull/2160