Disable buffering for raw_null_ostream()

[mehdi_amini]

Summary

There is no need to buffer the nulls() output.
Moreover it kept a shared buffer, and made using nulls() not possible
in a multi-threaded environment.

[mehdi_amini]

Take Duncan review into account

[mehdi_amini]

Split refactoring in D10885, leaving only the functional change for nulls() here.

include/llvm/Support/circular_raw_ostream.h

public:
     /// buffering to be happening underneath it.
     ///
     /// "Owns" tells the circular_raw_ostream whether it is
     /// responsible for managing the held stream, doing memory
     /// management of it, etc.
     ///
     circular_raw_ostream(raw_ostream &Stream, const char *Header,
                          size_t BuffSize = 0, bool Owns = REFERENCE_ONLY)
-        : raw_ostream(/*unbuffered*/ true), TheStream(nullptr),
+        : raw_ostream(BufferKind::Unbuffered), TheStream(nullptr),
           OwnsStream(Owns), BufferSize(BuffSize), BufferArray(nullptr),
           Filled(false), Banner(Header) {
       if (BufferSize != 0)
         BufferArray = new char[BufferSize];
       Cur = BufferArray;
       setStream(Stream, Owns);
     }
 

include/llvm/Support/raw_ostream.h

private:
   /// InternalBuffer); otherwise the buffer has been set via SetBuffer and is
   /// managed by the subclass.
   ///
   /// If a subclass installs an external buffer using SetBuffer then it can wait
   /// for a \see write_impl() call to handle the data which has been put into
   /// this buffer.
   char *OutBufStart, *OutBufEnd, *OutBufCur;
 
-  enum BufferKind {
-    Unbuffered = 0,
-    InternalBuffer,
-    ExternalBuffer
-  } BufferMode;
+public:
+  /// Specify the current state of the stream.
+  enum class BufferKind { Unbuffered, InternalBuffer, ExternalBuffer };
+
+private:
+  BufferKind BufferMode;
 
 public:
   // color order matches ANSI escape sequence, don't change
   enum Colors {
     BLACK=0,
     RED,
     GREEN,
     YELLOW,
     BLUE,
     MAGENTA,
     CYAN,
     WHITE,
     SAVEDCOLOR
   };
 
-  explicit raw_ostream(bool unbuffered = false)
-      : BufferMode(unbuffered ? Unbuffered : InternalBuffer) {
+  explicit raw_ostream(BufferKind BufferMode = BufferKind::InternalBuffer)
+      : BufferMode(BufferMode) {
     // Start out ready to flush.
     OutBufStart = OutBufEnd = OutBufCur = nullptr;
   }
 
   virtual ~raw_ostream();
 
   /// tell - Return the current offset with the file.
   uint64_t tell() const { return current_pos() + GetNumBytesInBuffer(); }
 
   //===--------------------------------------------------------------------===//
   // Configuration Interface
   //===--------------------------------------------------------------------===//
 
   /// Set the stream to be buffered, with an automatically determined buffer
   /// size.
   void SetBuffered();
 
   /// Set the stream to be buffered, using the specified buffer size.
   void SetBufferSize(size_t Size) {
     flush();
-    SetBufferAndMode(new char[Size], Size, InternalBuffer);
+    SetBufferAndMode(new char[Size], Size, BufferKind::InternalBuffer);
   }
 
   size_t GetBufferSize() const {
     // If we're supposed to be buffered but haven't actually gotten around
     // to allocating the buffer yet, return the value that would be used.
-    if (BufferMode != Unbuffered && OutBufStart == nullptr)
+    if (BufferMode != BufferKind::Unbuffered && OutBufStart == nullptr)
       return preferred_buffer_size();
 
     // Otherwise just return the size of the allocated buffer.
     return OutBufEnd - OutBufStart;
   }
 
   /// Set the stream to be unbuffered. When unbuffered, the stream will flush
   /// after every write. This routine will also flush the buffer immediately
   /// when the stream is being set to unbuffered.
   void SetUnbuffered() {
     flush();
-    SetBufferAndMode(nullptr, 0, Unbuffered);
+    SetBufferAndMode(nullptr, 0, BufferKind::Unbuffered);
   }
 
   size_t GetNumBytesInBuffer() const {
     return OutBufCur - OutBufStart;
   }
 
   //===--------------------------------------------------------------------===//
   // Data Output Interface
private:
   /// currently in the buffer.
   virtual uint64_t current_pos() const = 0;
 
 protected:
   /// Use the provided buffer as the raw_ostream buffer. This is intended for
   /// use only by subclasses which can arrange for the output to go directly
   /// into the desired output buffer, instead of being copied on each flush.
   void SetBuffer(char *BufferStart, size_t Size) {
-    SetBufferAndMode(BufferStart, Size, ExternalBuffer);
+    SetBufferAndMode(BufferStart, Size, BufferKind::ExternalBuffer);
   }
 
   /// Return an efficient buffer size for the underlying output mechanism.
   virtual size_t preferred_buffer_size() const;
 
   /// Return the beginning of the current stream buffer, or 0 if the stream is
   /// unbuffered.
   const char *getBufferStart() const { return OutBufStart; }

 
 /// An abstract base class for streams implementations that also support a
 /// pwrite operation. This is usefull for code that can mostly stream out data,
 /// but needs to patch in a header that needs to know the output size.
 class raw_pwrite_stream : public raw_ostream {
   virtual void pwrite_impl(const char *Ptr, size_t Size, uint64_t Offset) = 0;
 
 public:
-  explicit raw_pwrite_stream(bool Unbuffered = false)
+  explicit raw_pwrite_stream(BufferKind Unbuffered = BufferKind::InternalBuffer)
       : raw_ostream(Unbuffered) {}
   void pwrite(const char *Ptr, size_t Size, uint64_t Offset) {
 #ifndef NDBEBUG
     uint64_t Pos = tell();
     // /dev/null always reports a pos of 0, so we cannot perform this check
     // in that case.
     if (Pos)
       assert(Size + Offset <= Pos && "We don't support extending the stream");
public:
   /// itself to own the file descriptor. In particular, it will close the
   /// file descriptor when it is done (this is necessary to detect
   /// output errors).
   raw_fd_ostream(StringRef Filename, std::error_code &EC,
                  sys::fs::OpenFlags Flags);
 
   /// FD is the file descriptor that this writes to.  If ShouldClose is true,
   /// this closes the file when the stream is destroyed.
-  raw_fd_ostream(int fd, bool shouldClose, bool unbuffered=false);
+  raw_fd_ostream(int fd, bool shouldClose,
+                 BufferKind Unbuffered = BufferKind::InternalBuffer);
 
   ~raw_fd_ostream() override;
 
   /// Manually flush the stream and close the file. Note that this does not call
   /// fsync.
   void close();
 
   bool supportsSeeking() { return SupportsSeeking; }
class raw_null_ostream : public raw_pwrite_stream {
   void write_impl(const char *Ptr, size_t size) override;
   void pwrite_impl(const char *Ptr, size_t Size, uint64_t Offset) override;
 
   /// Return the current position within the stream, not counting the bytes
   /// currently in the buffer.
   uint64_t current_pos() const override;
 
 public:
-  explicit raw_null_ostream() {}
+  explicit raw_null_ostream() : raw_pwrite_stream(BufferKind::Unbuffered) {}
   ~raw_null_ostream() override;
 };
 
 class buffer_ostream : public raw_svector_ostream {
   raw_ostream &OS;
   SmallVector<char, 0> Buffer;
 
 public:

lib/Bitcode/Writer/BitWriter.cpp

if (EC)
     return -1;
 
   WriteBitcodeToFile(unwrap(M), OS);
   return 0;
 }
 
 int LLVMWriteBitcodeToFD(LLVMModuleRef M, int FD, int ShouldClose,
                          int Unbuffered) {
-  raw_fd_ostream OS(FD, ShouldClose, Unbuffered);
+  raw_ostream::BufferKind Mode = Unbuffered
+                                     ? raw_ostream::BufferKind::Unbuffered
+                                     : raw_ostream::BufferKind::InternalBuffer;
+  raw_fd_ostream OS(FD, ShouldClose, Mode);
 
   WriteBitcodeToFile(unwrap(M), OS);
   return 0;
 }
 
 int LLVMWriteBitcodeToFileHandle(LLVMModuleRef M, int FileHandle) {
   return LLVMWriteBitcodeToFD(M, FileHandle, true, false);
 }
 
 LLVMMemoryBufferRef LLVMWriteBitcodeToMemoryBuffer(LLVMModuleRef M) {
   std::string Data;
   raw_string_ostream OS(Data);
 
   WriteBitcodeToFile(unwrap(M), OS);
   return wrap(MemoryBuffer::getMemBufferCopy(OS.str()).release());
 }

lib/Support/raw_ostream.cpp

 using namespace llvm;
 
 raw_ostream::~raw_ostream() {
   // raw_ostream's subclasses should take care to flush the buffer
   // in their destructors.
   assert(OutBufCur == OutBufStart &&
          "raw_ostream destructor called with non-empty buffer!");
 
-  if (BufferMode == InternalBuffer)
+  if (BufferMode == BufferKind::InternalBuffer)
     delete [] OutBufStart;
 }
 
 // An out of line virtual method to provide a home for the class vtable.
 void raw_ostream::handle() {}
 
 size_t raw_ostream::preferred_buffer_size() const {
   // BUFSIZ is intended to be a reasonable default.
   return BUFSIZ;
 }
 
 void raw_ostream::SetBuffered() {
   // Ask the subclass to determine an appropriate buffer size.
   if (size_t Size = preferred_buffer_size())
     SetBufferSize(Size);
   else
     // It may return 0, meaning this stream should be unbuffered.
     SetUnbuffered();
 }
 
 void raw_ostream::SetBufferAndMode(char *BufferStart, size_t Size,
                                    BufferKind Mode) {
-  assert(((Mode == Unbuffered && !BufferStart && Size == 0) ||
-          (Mode != Unbuffered && BufferStart && Size != 0)) &&
+  assert(((Mode == BufferKind::Unbuffered && !BufferStart && Size == 0) ||
+          (Mode != BufferKind::Unbuffered && BufferStart && Size != 0)) &&
          "stream must be unbuffered or have at least one byte");
   // Make sure the current buffer is free of content (we can't flush here; the
   // child buffer management logic will be in write_impl).
   assert(GetNumBytesInBuffer() == 0 && "Current buffer is non-empty!");
 
-  if (BufferMode == InternalBuffer)
+  if (BufferMode == BufferKind::InternalBuffer)
     delete [] OutBufStart;
   OutBufStart = BufferStart;
   OutBufEnd = OutBufStart+Size;
   OutBufCur = OutBufStart;
   BufferMode = Mode;
 
   assert(OutBufStart <= OutBufEnd && "Invalid size!");
 }
void raw_ostream::flush_nonempty() {
   OutBufCur = OutBufStart;
   write_impl(OutBufStart, Length);
 }
 
 raw_ostream &raw_ostream::write(unsigned char C) {
   // Group exceptional cases into a single branch.
   if (LLVM_UNLIKELY(OutBufCur >= OutBufEnd)) {
     if (LLVM_UNLIKELY(!OutBufStart)) {
-      if (BufferMode == Unbuffered) {
+      if (BufferMode == BufferKind::Unbuffered) {
         write_impl(reinterpret_cast<char*>(&C), 1);
         return *this;
       }
       // Set up a buffer and start over.
       SetBuffered();
       return write(C);
     }
 
     flush_nonempty();
   }
 
   *OutBufCur++ = C;
   return *this;
 }
 
 raw_ostream &raw_ostream::write(const char *Ptr, size_t Size) {
   // Group exceptional cases into a single branch.
   if (LLVM_UNLIKELY(size_t(OutBufEnd - OutBufCur) < Size)) {
     if (LLVM_UNLIKELY(!OutBufStart)) {
-      if (BufferMode == Unbuffered) {
+      if (BufferMode == BufferKind::Unbuffered) {
         write_impl(Ptr, Size);
         return *this;
       }
       // Set up a buffer and start over.
       SetBuffered();
       return write(Ptr, Size);
     }
 

 }
 
 raw_fd_ostream::raw_fd_ostream(StringRef Filename, std::error_code &EC,
                                sys::fs::OpenFlags Flags)
     : raw_fd_ostream(getFD(Filename, EC, Flags), true) {}
 
 /// FD is the file descriptor that this writes to.  If ShouldClose is true, this
 /// closes the file when the stream is destroyed.
-raw_fd_ostream::raw_fd_ostream(int fd, bool shouldClose, bool unbuffered)
-    : raw_pwrite_stream(unbuffered), FD(fd), ShouldClose(shouldClose),
-      Error(false), UseAtomicWrites(false) {
+raw_fd_ostream::raw_fd_ostream(int fd, bool shouldClose, BufferKind Mode)
+    : raw_pwrite_stream(Mode), FD(fd), ShouldClose(shouldClose), Error(false),
+      UseAtomicWrites(false) {
   if (FD < 0 ) {
     ShouldClose = false;
     return;
   }
 
   // Get the starting position.
   off_t loc = ::lseek(FD, 0, SEEK_CUR);
 #ifdef LLVM_ON_WIN32
raw_ostream &llvm::outs() {
   assert(!EC);
   return S;
 }
 
 /// errs() - This returns a reference to a raw_ostream for standard error.
 /// Use it like: errs() << "foo" << "bar";
 raw_ostream &llvm::errs() {
   // Set standard error to be unbuffered by default.
-  static raw_fd_ostream S(STDERR_FILENO, false, true);
+  static raw_fd_ostream S(STDERR_FILENO, false,
+                          raw_ostream::BufferKind::Unbuffered);
   return S;
 }
 
 /// nulls() - This returns a reference to a raw_ostream which discards output.
 raw_ostream &llvm::nulls() {
   static raw_null_ostream S;
   return S;
 }

unittests/Support/MemoryBufferTest.cpp

 
 TEST_F(MemoryBufferTest, NullTerminator4K) {
   // Test that a file with size that is a multiple of the page size can be null
   // terminated correctly by MemoryBuffer.
   int TestFD;
   SmallString<64> TestPath;
   sys::fs::createTemporaryFile("MemoryBufferTest_NullTerminator4K", "temp",
                                TestFD, TestPath);
-  raw_fd_ostream OF(TestFD, true, /*unbuffered=*/true);
+  raw_fd_ostream OF(TestFD, true, raw_ostream::BufferKind::Unbuffered);
   for (unsigned i = 0; i < 4096 / 16; ++i) {
     OF << "0123456789abcdef";
   }
   OF.close();
 
   ErrorOr<OwningBuffer> MB = MemoryBuffer::getFile(TestPath.c_str());
   std::error_code EC = MB.getError();
   ASSERT_FALSE(EC);
void MemoryBufferTest::testGetOpenFileSlice(bool Reopen) {
   // the usage of memory mapping.
   int TestFD;
   SmallString<64> TestPath;
   // Create a temporary file and write data into it.
   sys::fs::createTemporaryFile("prefix", "temp", TestFD, TestPath);
   // OF is responsible for closing the file; If the file is not
   // reopened, it will be unbuffered so that the results are
   // immediately visible through the fd.
-  raw_fd_ostream OF(TestFD, true, !Reopen);
+  raw_fd_ostream OF(TestFD, true, Reopen
+                                      ? raw_ostream::BufferKind::InternalBuffer
+                                      : raw_ostream::BufferKind::Unbuffered);
   for (int i = 0; i < 60000; ++i) {
     OF << "0123456789";
   }
 
   if (Reopen) {
     OF.close();
     EXPECT_FALSE(sys::fs::openFileForRead(TestPath.c_str(), TestFD));
   }

 }
 
 
 TEST_F(MemoryBufferTest, slice) {
   // Create a file that is six pages long with different data on each page.
   int FD;
   SmallString<64> TestPath;
   sys::fs::createTemporaryFile("MemoryBufferTest_Slice", "temp", FD, TestPath);
-  raw_fd_ostream OF(FD, true, /*unbuffered=*/true);
+  raw_fd_ostream OF(FD, true, raw_ostream::BufferKind::Unbuffered);
   for (unsigned i = 0; i < 0x2000 / 8; ++i) {
     OF << "12345678";
   }
   for (unsigned i = 0; i < 0x2000 / 8; ++i) {
     OF << "abcdefgh";
   }
   for (unsigned i = 0; i < 0x2000 / 8; ++i) {
     OF << "ABCDEFGH";

unittests/Support/SpecialCaseListTest.cpp

std::unique_ptr<SpecialCaseList> makeSpecialCaseList(StringRef List) {
     assert(Error == "");
     return SCL;
   }
 
   std::string makeSpecialCaseListFile(StringRef Contents) {
     int FD;
     SmallString<64> Path;
     sys::fs::createTemporaryFile("SpecialCaseListTest", "temp", FD, Path);
-    raw_fd_ostream OF(FD, true, true);
+    raw_fd_ostream OF(FD, true, raw_ostream::BufferKind::Unbuffered);
     OF << Contents;
     OF.close();
     return Path.str();
   }
 };
 
 TEST_F(SpecialCaseListTest, Basic) {
   std::unique_ptr<SpecialCaseList> SCL =