RFR: JDK-8260030: Improve stringStream buffer handling [v3]

Thomas Stuefe stuefe at openjdk.java.net
Fri Jan 22 09:04:15 UTC 2021


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

Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:

  Use ! instead of false for is_fixed

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2160/files
  - new: https://git.openjdk.java.net/jdk/pull/2160/files/cced1e79..caf62f09

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2160&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2160&range=01-02

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2160.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2160/head:pull/2160

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


More information about the hotspot-dev mailing list