diff -r 9846af5a0949 src/hotspot/share/utilities/ostream.cpp --- a/src/hotspot/share/utilities/ostream.cpp Mon Apr 19 12:47:46 2021 +0200 +++ b/src/hotspot/share/utilities/ostream.cpp Wed Apr 21 09:51:48 2021 +0200 @@ -307,46 +307,72 @@ } } -stringStream::stringStream(size_t initial_size) : outputStream() { - buffer_length = initial_size; - buffer = NEW_C_HEAP_ARRAY(char, buffer_length, mtInternal); - buffer_pos = 0; - buffer_fixed = false; +stringStream::stringStream(size_t initial_capacity) : + outputStream(), + _buffer(_small_buffer), + _written(0), + _capacity(sizeof(_small_buffer)), + _is_fixed(false) +{ + if (initial_capacity > _capacity) { + grow(initial_capacity); + } zero_terminate(); } // useful for output to fixed chunks of memory, such as performance counters -stringStream::stringStream(char* fixed_buffer, size_t fixed_buffer_size) : outputStream() { - buffer_length = fixed_buffer_size; - buffer = fixed_buffer; - buffer_pos = 0; - buffer_fixed = true; +stringStream::stringStream(char* fixed_buffer, size_t fixed_buffer_size) : + outputStream(), + _buffer(fixed_buffer), + _written(0), + _capacity(fixed_buffer_size), + _is_fixed(true) +{ zero_terminate(); } +// Grow backing buffer to desired capacity. Don't call for fixed buffers +void stringStream::grow(size_t new_capacity) { + assert(!_is_fixed, "Don't call for caller provided buffers"); + assert(new_capacity > _capacity, "Sanity"); + assert(new_capacity > sizeof(_small_buffer), "Sanity"); + if (_buffer == _small_buffer) { + _buffer = NEW_C_HEAP_ARRAY(char, new_capacity, mtInternal); + _capacity = new_capacity; + if (_written > 0) { + ::memcpy(_buffer, _small_buffer, _written); + } + zero_terminate(); + } else { + _buffer = REALLOC_C_HEAP_ARRAY(char, _buffer, new_capacity, mtInternal); + _capacity = new_capacity; + } +} + void stringStream::write(const char* s, size_t len) { - size_t write_len = len; // number of non-null bytes to write - size_t end = buffer_pos + len + 1; // position after write and final '\0' - if (end > buffer_length) { - if (buffer_fixed) { - // if buffer cannot resize, silently truncate - end = buffer_length; - write_len = end - buffer_pos - 1; // leave room for the final '\0' - } else { - // For small overruns, double the buffer. For larger ones, - // increase to the requested size. - if (end < buffer_length * 2) { - end = buffer_length * 2; - } - buffer = REALLOC_C_HEAP_ARRAY(char, buffer, end, mtInternal); - buffer_length = end; + assert(_capacity >= _written + 1, "Sanity"); + if (len == 0) { + return; + } + const size_t reasonable_max_len = 1 * G; + if (len >= reasonable_max_len) { + assert(false, "bad length? (" SIZE_FORMAT ")", len); + return; + } + size_t write_len = 0; + if (_is_fixed) { + write_len = MIN2(len, _capacity - _written - 1); + } else { + write_len = len; + size_t needed = _written + len + 1; + if (needed > _capacity) { + grow(MAX2(needed, _capacity * 2)); } } - // invariant: buffer is always null-terminated - guarantee(buffer_pos + write_len + 1 <= buffer_length, "stringStream oob"); + assert(_written + write_len + 1 <= _capacity, "stringStream oob"); if (write_len > 0) { - memcpy(buffer + buffer_pos, s, write_len); - buffer_pos += write_len; + ::memcpy(_buffer + _written, s, write_len); + _written += write_len; zero_terminate(); } @@ -357,26 +383,27 @@ } void stringStream::zero_terminate() { - assert(buffer != NULL && - buffer_pos < buffer_length, "sanity"); - buffer[buffer_pos] = '\0'; + assert(_buffer != NULL && + _written < _capacity, "sanity"); + _buffer[_written] = '\0'; } void stringStream::reset() { - buffer_pos = 0; _precount = 0; _position = 0; + _written = 0; _precount = 0; _position = 0; + _newlines = 0; zero_terminate(); } char* stringStream::as_string() const { - char* copy = NEW_RESOURCE_ARRAY(char, buffer_pos + 1); - strncpy(copy, buffer, buffer_pos); - copy[buffer_pos] = 0; // terminating null + char* copy = NEW_RESOURCE_ARRAY(char, _written + 1); + ::memcpy(copy, _buffer, _written); + copy[_written] = 0; // terminating null return copy; } stringStream::~stringStream() { - if (buffer_fixed == false && buffer != NULL) { - FREE_C_HEAP_ARRAY(char, buffer); + if (!_is_fixed && _buffer != _small_buffer) { + FREE_C_HEAP_ARRAY(char, _buffer); } } diff -r 9846af5a0949 src/hotspot/share/utilities/ostream.hpp --- a/src/hotspot/share/utilities/ostream.hpp Mon Apr 19 12:47:46 2021 +0200 +++ b/src/hotspot/share/utilities/ostream.hpp Wed Apr 21 09:51:48 2021 +0200 @@ -87,7 +87,6 @@ // sizing int width() const { return _width; } int position() const { return _position; } - int newlines() const { return _newlines; } julong count() const { return _precount + _position; } void set_count(julong count) { _precount = count - _position; } void set_position(int pos) { _position = pos; } @@ -192,11 +191,14 @@ // for writing to strings; buffer will expand automatically. // Buffer will always be zero-terminated. class stringStream : public outputStream { - protected: - char* buffer; - size_t buffer_pos; - size_t buffer_length; - bool buffer_fixed; + char* _buffer; + size_t _written; // Number of characters written, excluding termin. zero + size_t _capacity; + const bool _is_fixed; + char _small_buffer[48]; + + // Grow backing buffer to desired capacity. + void grow(size_t new_capacity); // zero terminate at buffer_pos. void zero_terminate(); @@ -204,7 +206,7 @@ public: // Create a stringStream using an internal buffer of initially initial_bufsize size; // will be enlarged on demand. There is no maximum cap. - stringStream(size_t initial_bufsize = 256); + stringStream(size_t initial_capacity = 0); // Creates a stringStream using a caller-provided buffer. Will truncate silently if // it overflows. stringStream(char* fixed_buffer, size_t fixed_buffer_size); @@ -212,8 +214,8 @@ virtual void write(const char* c, size_t len); // Return number of characters written into buffer, excluding terminating zero and // subject to truncation in static buffer mode. - size_t size() const { return buffer_pos; } - const char* base() const { return buffer; } + size_t size() const { return _written; } + const char* base() const { return _buffer; } void reset(); char* as_string() const; }; diff -r 9846af5a0949 test/hotspot/gtest/utilities/test_ostream.cpp --- a/test/hotspot/gtest/utilities/test_ostream.cpp Mon Apr 19 12:47:46 2021 +0200 +++ b/test/hotspot/gtest/utilities/test_ostream.cpp Wed Apr 21 09:51:48 2021 +0200 @@ -31,7 +31,7 @@ #include "unittest.hpp" -static size_t print_lorem(outputStream* st, bool short_len) { +static size_t print_lorem(outputStream* st) { // Create a ResourceMark just to make sure the stream does not use ResourceArea ResourceMark rm; static const char* const lorem = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, " @@ -39,12 +39,9 @@ "risus sed vulputate odio ut enim blandit. Amet risus nullam eget felis eget. Viverra " "orci sagittis eu volutpat odio facilisis mauris sit. Erat velit scelerisque in dictum non."; static const size_t len_lorem = strlen(lorem); - size_t len; - if (short_len) { - len = os::random() % 10; - } else { - len = MAX2(1, (int)(os::random() % len_lorem)); - } + // Randomly alternate between short and long writes at a ratio of 9:1. + const bool short_write = (os::random() % 10) > 0; + const size_t len = os::random() % (short_write ? 10 : len_lorem); st->write(lorem, len); return len; } @@ -53,12 +50,11 @@ ASSERT_EQ(ss->base()[ss->size()], '\0'); } - -static void do_test_stringStream(stringStream* ss, bool short_len, size_t expected_cap) { +static void do_test_stringStream(stringStream* ss, size_t expected_cap) { test_stringStream_is_zero_terminated(ss); size_t written = 0; for (int i = 0; i < 1000; i ++) { - written += print_lorem(ss, short_len); + written += print_lorem(ss); if (expected_cap > 0 && written >= expected_cap) { ASSERT_EQ(ss->size(), expected_cap - 1); } else { @@ -73,14 +69,18 @@ test_stringStream_is_zero_terminated(ss); } -TEST_VM(ostream, stringStream_dynamic_realloc_1) { - stringStream ss(2); // dynamic buffer with very small starting size - do_test_stringStream(&ss, false, 0); +TEST_VM(ostream, stringStream_dynamic_start_with_internal_buffer) { + stringStream ss; + do_test_stringStream(&ss, 0); + ss.reset(); + do_test_stringStream(&ss, 0); } -TEST_VM(ostream, stringStream_dynamic_realloc_2) { - stringStream ss(2); // dynamic buffer with very small starting size - do_test_stringStream(&ss, true, 0); +TEST_VM(ostream, stringStream_dynamic_start_with_malloced_buffer) { + stringStream ss(128); + do_test_stringStream(&ss, 0); + ss.reset(); + do_test_stringStream(&ss, 0); } TEST_VM(ostream, stringStream_static) { @@ -89,7 +89,7 @@ *canary_at = 'X'; size_t stream_buf_size = sizeof(buffer) - 1; stringStream ss(buffer, stream_buf_size); - do_test_stringStream(&ss, false, stream_buf_size); + do_test_stringStream(&ss, stream_buf_size); ASSERT_EQ(*canary_at, 'X'); // canary } @@ -101,7 +101,7 @@ bufferedStream bs(buf, stream_buf_size); size_t written = 0; for (int i = 0; i < 100; i ++) { - written += print_lorem(&bs, true); + written += print_lorem(&bs); if (written < stream_buf_size) { ASSERT_EQ(bs.size(), written); } else { @@ -116,7 +116,7 @@ size_t written = 0; // The max cap imposed is 100M, we should be safely below this in this test. for (int i = 0; i < 10; i ++) { - written += print_lorem(&bs, true); + written += print_lorem(&bs); ASSERT_EQ(bs.size(), written); } } @@ -130,7 +130,7 @@ // Note that this will assert in debug builds which is the expected behavior. size_t expected_cap_at = 100 * M; for (int i = 0; i < 10000000; i ++) { - written += print_lorem(&bs, false); + written += print_lorem(&bs); if (written < expected_cap_at) { ASSERT_EQ(bs.size(), written); } else {