[Support] Make the default chunk size of raw_fd_ostream to 1 GiB.

[ruiu]

Summary

Previously, we call write(2) for each 32767 byte chunk. That is not
efficient because Linux can handle much larger write requests.
This patch changes the chunk size on Linux to 1 GiB.

This patch doesn't change the existing behavior for Windows.

[zturner]

llvm/lib/Support/raw_ostream.cpp

590–592

The comment here doesn't make sense to me. It says "Writing a large size of output to Windows console", but then the code only limits the write size if it's not a TTY. Isn't this check inverted? It should be setting the max write size if it is a tty.

Furthermore, the documentation for WriteConsole doesn't say anything about Windows 8, it just says the buffer is at most 64k. It even says that the buffer could be smaller, but how much smaller depends on heap usage. So for all we know, it might not even be 32K.

Granted, you're only changing the non-Windows code path, but this just looks wrong to me, in case you're interested in fixing it.

[compnerd]

llvm/lib/Support/raw_ostream.cpp

590–592

Its a conversion trick (int to boolean) via double negation.

[ruiu]

llvm/lib/Support/raw_ostream.cpp

590–592

Actually it is !!, so it checks if isatty && Windows7OrOlder.

But I agree that this is confusing. I'll just remove !! as it doesn't seem necessary.

[zturner]

llvm/lib/Support/raw_ostream.cpp

590–592

Oh, duhh.. Now I feel stupid. I use the !! myself too, it just wasn't registering with me for some reason. The whole Windows 8 thing still seems dubious to me, since there's no reference (e.g. msdn link) about why this is specific to Windows 7 and below.

[krytarowski]

llvm/lib/Support/raw_ostream.cpp

583–586

Am I looking correctly that we enforce Linux behavior to every UNIX?

[ruiu]

llvm/lib/Support/raw_ostream.cpp

590–592

I think we can just remove !RunningWindows8OrGreater(), as limiting write size to 32 KiB is not that inefficient, and when we are writing to a console, console's speed is a bottleneck anyway. But that should be done in a separate patch.

[ruiu]

llvm/lib/Support/raw_ostream.cpp

583–586

Yes. Compared to writing 1 GiB data, the overhead of system call is negligible, so I kept it simple.

[krytarowski]

llvm/lib/Support/raw_ostream.cpp

583–586

Keeping it simple is fine, but I prompt to make this comment more aware of existence of other OSes... or even better just enable it on Linux, it might silently break others like Linux for >2GB.

[ruiu]

• Do it only for Linux.

[ruiu]

llvm/lib/Support/raw_ostream.cpp

583–586

Sure.

[krytarowski]

llvm/lib/Support/raw_ostream.cpp

580

If the value of nbyte is greater than {SSIZE_MAX}, the result is implementation-defined.

http://pubs.opengroup.org/onlinepubs/9699919799/functions/write.html#tag_16_685

[ruiu]

llvm/lib/Support/raw_ostream.cpp

580

Ah. Then, maybe that's what Saleem observed? The existing code could make a write request that is larger than SSIZE_MAX (= 2 GiB - 1) on a 32-bit machine.

If so, we can just remove the ifdef for Linux and use SSIZE_T as the default MaxWriteSize.

[krytarowski]

llvm/lib/Support/raw_ostream.cpp

580

SSIZE_MAX is POSIX only (unless it was added to C++, but I don't see a reference).

NetBSD specific behavior allows length up to SSIZE_MAX:

	/*
	 * Writes return ssize_t because -1 is returned on error.  Therefore
	 * we must restrict the length to SSIZE_MAX to avoid garbage return
	 * values.
	 */
	if (auio.uio_resid > SSIZE_MAX) {
		error = EINVAL;
		goto out;
}

• src/sys/kern/sys_generic.c

[krytarowski]

llvm/lib/Support/raw_ostream.cpp

655

The same problem here. NetBSD disallows length larger than SSIZE_MAX and it will return EINVAL.

[ruiu]

• Limit the maximum write size to SSIZE_MAX.

[ruiu]

llvm/lib/Support/raw_ostream.cpp

580

I'm not really sure if we want to deal with limitations of SSIZE_MAX instead of just calling write for each 1 GiB of data, but I addressed your concern in the new patch.

655

This is not a call of ::write but write member function defined in the same class.

llvm/lib/Support/raw_ostream.cpp

#endif
   if (has_error())
     report_fatal_error("IO failure on output stream: " + error().message(),
                        /*GenCrashDiag=*/false);
 }
 
 void raw_fd_ostream::write_impl(const char *Ptr, size_t Size) {
   assert(FD >= 0 && "File already closed.");
   pos += Size;
+  size_t MaxWriteSize = SIZE_MAX;
 
-#ifndef LLVM_ON_WIN32
 #if defined(__linux__)
-  bool ShouldWriteInChunks = true;
-#else
-  bool ShouldWriteInChunks = false;
-#endif
-#else
+  // It is observed that Linux returns EINVAL for a very large write (>2G).
+  // Make it a reasonably small value.
+  MaxWriteSize = 1024 * 1024 * 1024;
+#elif defined(LLVM_ON_WIN32)
   // Writing a large size of output to Windows console returns ENOMEM. It seems
   // that, prior to Windows 8, WriteFile() is redirecting to WriteConsole(), and
   // the latter has a size limit (66000 bytes or less, depending on heap usage).
-  bool ShouldWriteInChunks = !!::_isatty(FD) && !RunningWindows8OrGreater();
+  if (::_isatty(FD) && !RunningWindows8OrGreater())
+    MaxWriteSize = 32767;
 #endif
 
   do {
-    size_t ChunkSize = Size;
-    if (ChunkSize > 32767 && ShouldWriteInChunks)
-        ChunkSize = 32767;
-
+    size_t ChunkSize = std::min(Size, MaxWriteSize);
     ssize_t ret = ::write(FD, Ptr, ChunkSize);
 
     if (ret < 0) {
       // If it's a recoverable error, swallow it and retry the write.
       //
       // Ideally we wouldn't ever see EAGAIN or EWOULDBLOCK here, since
       // raw_ostream isn't designed to do non-blocking I/O. However, some
       // programs, such as old versions of bjam, have mistakenly used
if (pos == (uint64_t)-1)
     error_detected(std::error_code(errno, std::generic_category()));
   return pos;
 }
 
 void raw_fd_ostream::pwrite_impl(const char *Ptr, size_t Size,
                                  uint64_t Offset) {
   uint64_t Pos = tell();
   seek(Offset);
   write(Ptr, Size);
   seek(Pos);
 }
 
 size_t raw_fd_ostream::preferred_buffer_size() const {
 #if !defined(_MSC_VER) && !defined(__MINGW32__) && !defined(__minix)
   // Windows and Minix have no st_blksize.
   assert(FD >= 0 && "File not yet open!");
   struct stat statbuf;