[Support] Make raw_string_ostream unbuffered

[lebedev.ri]

Summary

In D58580 i have noted that llvm::to_string() is a memory hog.
It uses raw_string_ostream, and since it was buffered,
every raw_string_ostream had a cost of BUFSIZ bytes
(which is 8192 at least here). So every llvm::to_string()
call, even to just print an int, costed 8192 bytes.

In D58580, getting rid of that buffering had significant
performance and memory consumption improvements for llvm-xray convert.

Similarly, in D58580 @rnk pointed out that the raw_svector_ostream
is already unbuffered, and write_unsigned_impl and friends
do internal buffering. So it should be ok to also make
raw_string_ostream unbuffered.

Here, i don't have any perf measurements.
Another letdown is that i'm leaving a loose-end - not deleting the flush() method.
I don't expect that cleanup to be anything more than just fixing every
new compiler error, but i'm presently unable to do that.

[rnk]

lgtm

Leaving flush alone is fine.

include/llvm/Support/raw_ostream.h

497 ↗

(On Diff #188237)

Huh, I suppose that is nicer than calling SetUnbuffered.

[lebedev.ri]

In D58643#1409649, @rnk wrote:

lgtm

Thank you for the review.

Leaving flush alone is fine.

The problem being that i still haven't migrated to the monorepo.
Hm, actually, i think i can do it (well, except lldb), simply by doing all the changes,
committing them, and then committing the removal of the method itself last.
I'll look into that later.

include/llvm/Support/raw_ostream.h

497 ↗

(On Diff #188237)

Thanks

[lebedev.ri]

Uhm, but if we disable buffering, and cripple flush(), shouldn't we also make it impossible to turn the buffering back on?
Existing test fails with this change, and if i add similar test with SmallString/raw_svector_ostream it also fails in trunk.

TEST(raw_ostreamTest, TinyBuffer) {
  std::string Str;
  raw_string_ostream OS(Str);
  OS.SetBufferSize(1);
  OS << "hello";
  OS << 1;
  OS << 'w' << 'o' << 'r' << 'l' << 'd';
  EXPECT_EQ("hello1world", OS.str());
}

TEST(raw_svector_ostreamTest, TinyBuffer) {
  llvm::SmallString<0> Str;
  raw_svector_ostream OS(Str);
  OS.SetBufferSize(1);
  OS << "hello";
  OS << 1;
  OS << 'w' << 'o' << 'r' << 'l' << 'd';
  EXPECT_EQ("hello1world", OS.str());
}

$ unittests/Support/SupportTests --gtest_filter=*TinyBuffer
Note: Google Test filter = *TinyBuffer
[==========] Running 2 tests from 2 test cases.
[----------] Global test environment set-up.
[----------] 1 test from raw_ostreamTest
[ RUN      ] raw_ostreamTest.TinyBuffer
/build/llvm/unittests/Support/raw_ostream_test.cpp:127: Failure
      Expected: "hello1world"
To be equal to: OS.str()
      Which is: "hello1worl"
[  FAILED  ] raw_ostreamTest.TinyBuffer (0 ms)
[----------] 1 test from raw_ostreamTest (0 ms total)

[----------] 1 test from raw_svector_ostreamTest
[ RUN      ] raw_svector_ostreamTest.TinyBuffer
/build/llvm/unittests/Support/raw_ostream_test.cpp:137: Failure
      Expected: "hello1world"
      Which is: 0x55cc2aeca73f
To be equal to: OS.str()
      Which is: { 'h' (104, 0x68), 'e' (101, 0x65), 'l' (108, 0x6C), 'l' (108, 0x6C), 'o' (111, 0x6F), '1' (49, 0x31), 'w' (119, 0x77), 'o' (111, 0x6F), 'r' (114, 0x72), 'l' (108, 0x6C) }
SupportTests: /build/llvm/lib/Support/raw_ostream.cpp:72: virtual llvm::raw_ostream::~raw_ostream(): Assertion `OutBufCur == OutBufStart && "raw_ostream destructor called with non-empty buffer!"' failed.
 #0 0x00007fb00164150a llvm::sys::PrintStackTrace(llvm::raw_ostream&) (/build/llvm-build-GCC-release/lib/libLLVMSupport.so.9svn+0x1b150a)
 #1 0x00007fb00163f2f4 llvm::sys::RunSignalHandlers() (/build/llvm-build-GCC-release/lib/libLLVMSupport.so.9svn+0x1af2f4)
 #2 0x00007fb00163f455 SignalHandler(int) (/build/llvm-build-GCC-release/lib/libLLVMSupport.so.9svn+0x1af455)
 #3 0x00007fb0014426e0 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x126e0)
 #4 0x00007fb000eff8bb gsignal (/lib/x86_64-linux-gnu/libc.so.6+0x378bb)
 #5 0x00007fb000eea535 abort (/lib/x86_64-linux-gnu/libc.so.6+0x22535)
 #6 0x00007fb000eea40f (/lib/x86_64-linux-gnu/libc.so.6+0x2240f)
 #7 0x00007fb000ef80f2 (/lib/x86_64-linux-gnu/libc.so.6+0x300f2)
 #8 0x00007fb00161fd40 llvm::raw_svector_ostream::~raw_svector_ostream() (/build/llvm-build-GCC-release/lib/libLLVMSupport.so.9svn+0x18fd40)
 #9 0x000055cc2b1fc226 (anonymous namespace)::raw_svector_ostreamTest_TinyBuffer_Test::TestBody() (unittests/Support/SupportTests+0x37c226)
#10 0x00007fb00140a98a testing::Test::Run() (/build/llvm-build-GCC-release/lib/libgtest.so.9svn+0x4298a)
#11 0x00007fb00140aad0 testing::TestInfo::Run() (/build/llvm-build-GCC-release/lib/libgtest.so.9svn+0x42ad0)
#12 0x00007fb00140ab95 testing::TestCase::Run() (/build/llvm-build-GCC-release/lib/libgtest.so.9svn+0x42b95)
#13 0x00007fb00140b05c testing::internal::UnitTestImpl::RunAllTests() (/build/llvm-build-GCC-release/lib/libgtest.so.9svn+0x4305c)
#14 0x00007fb00140b147 testing::UnitTest::Run() (/build/llvm-build-GCC-release/lib/libgtest.so.9svn+0x43147)
#15 0x00007fb001429161 main (/build/llvm-build-GCC-release/lib/libgtest_main.so.9svn+0x1161)
#16 0x00007fb000eec09b __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409b)
#17 0x000055cc2af6c02a _start (unittests/Support/SupportTests+0xec02a)
Aborted

[rnk]

In D58643#1409707, @lebedev.ri wrote:

Uhm, but if we disable buffering, and cripple flush(), shouldn't we also make it impossible to turn the buffering back on?
Existing test fails with this change, and if i add similar test with SmallString/raw_svector_ostream it also fails in trunk.

Can work around this by calling this->raw_ostream::flush() from str() and ~raw_string_ostream?

llvm/trunk/include/llvm/Support/raw_ostream.h

 raw_ostream &nulls();
 
 //===----------------------------------------------------------------------===//
 // Output Stream Adaptors
 //===----------------------------------------------------------------------===//
 
 /// A raw_ostream that writes to an std::string.  This is a simple adaptor
 /// class. This class does not encounter output errors.
+/// raw_string_ostream operates without a buffer, delegating all memory
+/// management to the std::string. Thus the std::string is always up-to-date,
+/// may be used directly and there is no need to call flush().
 class raw_string_ostream : public raw_ostream {
   std::string &OS;
 
   /// See raw_ostream::write_impl.
   void write_impl(const char *Ptr, size_t Size) override;
 
   /// Return the current position within the stream, not counting the bytes
   /// currently in the buffer.
   uint64_t current_pos() const override { return OS.size(); }
 
 public:
-  explicit raw_string_ostream(std::string &O) : OS(O) {}
+  explicit raw_string_ostream(std::string &O)
+      : raw_ostream(/*Unbuffered=*/true), OS(O) {}
+
   ~raw_string_ostream() override;
 
-  /// Flushes the stream contents to the target string and returns  the string's
-  /// reference.
+  // FIXME: uncomment and deal with the fallout.
+  // void flush() = delete;
+
+  /// Returns the string's reference.
   std::string& str() {
-    flush();
     return OS;
   }
 };
 
 /// A raw_ostream that writes to an SmallVector or SmallString.  This is a
 /// simple adaptor class. This class does not encounter output errors.
 /// raw_svector_ostream operates without a buffer, delegating all memory
 /// management to the SmallString. Thus the SmallString is always up-to-date,