[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

589–590 ↗

(On Diff #120925)

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

589–590 ↗

(On Diff #120925)

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

[ruiu]

llvm/lib/Support/raw_ostream.cpp

589–590 ↗

(On Diff #120925)

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

589–590 ↗

(On Diff #120925)

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

581 ↗

(On Diff #120925)

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

[ruiu]

llvm/lib/Support/raw_ostream.cpp

589–590 ↗

(On Diff #120925)

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

581 ↗

(On Diff #120925)

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

581 ↗

(On Diff #120925)

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

581 ↗

(On Diff #120925)

Sure.

[krytarowski]

llvm/lib/Support/raw_ostream.cpp

580 ↗

(On Diff #120929)

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 ↗

(On Diff #120929)

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 ↗

(On Diff #120929)

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 ↗

(On Diff #120929)

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 ↗

(On Diff #120929)

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 ↗

(On Diff #120929)

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

llvm/trunk/lib/Support/raw_ostream.cpp

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;
 
-#ifndef LLVM_ON_WIN32
+  // The maximum write size is limited to SSIZE_MAX because a write
+  // greater than SSIZE_MAX is implementation-defined in POSIX.
+  // Since SSIZE_MAX is not portable, we use SIZE_MAX >> 1 instead.
+  size_t MaxWriteSize = SIZE_MAX >> 1;
+
 #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