[Support] Revert posix_fallocate in resize_file

[MaskRay]

Summary

This reverts 3816c53f040cc6aa06425978dd504b0bd5b7899c and removes follow-up
fixups.

The original intention was to show error earlier (posix_fallocate time) than
later for ld.lld but it appears to cause some problems which make it not free.

• FreeBSD ZFS: EINVAL, not too bad.
• FreeBSD UFS: according to khng "devastatingly slow on freebsd because UFS on freebsd does not have preallocation support like illumos. It zero-fills."
• NetBSD: maybe EOPNOTSUPP
• Linux tmpfs: unless tmpfs is set up to use huge pages (requires CONFIG_TRANSPARENT_HUGE_PAGECACHE=y), I can consistently demonstrate ~300ms delay for a 1.4GiB output.
• Linux ext4: I don't measure any benefit, either backed by a hard disk or by a file in tmpfs.
• The current code organization of defined(HAVE_POSIX_FALLOCATE) costs us a macro dispatch for AIX.

I think we should just remove it. I think if posix_fallocate ever finds demonstrable benefit,
it is likely Linux specific and will not need HAVE_POSIX_FALLOCATE, and possibly opt-in by some specific programs.

In a filesystem with CoW and compression, the ENOSPC benefit may be lost as well.

[emaste]

it is likely Linux specific

Using it to prevent disk-full would equally apply to FreeBSD and other operating systems as Linux, on non-CoW filesystems.

Anyhow, the original motivation was early detection of disk-full not performance. I wouldn't really expect a performance benefit. I don't object to removing use of posix_fallocate() but as it wasn't intended as a performance improvement IMO removing it is not justified by lack of performance benefit.

[MaskRay]

In D115957#3200631, @emaste wrote:

it is likely Linux specific

Using it to prevent disk-full would equally apply to FreeBSD and other operating systems as Linux, on non-CoW filesystems.

Anyhow, the original motivation was early detection of disk-full not performance. I wouldn't really expect a performance benefit. I don't object to removing use of posix_fallocate() but as it wasn't intended as a performance improvement IMO removing it is not justified by lack of performance benefit.

It greatly harms performance on Linux tmpfs, 0.3s in my 5.2s link.

[joerg]

I agree that I don't think it generally helps all that much. Under memory pressure it can even result in additional writes.

[khng300]

My only suggestion is in future it could be platform-specific, like on macOS with fcntl(F_PREALLOCATE).

[MaskRay]

Thanks! I'll wait few days and push. If posix_fallocate is to be restored, I believe it should be passed explicitly as a flag, and there should be higher justification for lld/ELF.
I've noted this thing in https://maskray.me/blog/2021-12-19-why-isnt-ld.lld-faster#open-output-file

[emaste]

It greatly harms performance on Linux tmpfs, 0.3s in my 5.2s link.

Indeed, and I'm fine with it being removed.

[MaskRay]

In D115957#3203223, @emaste wrote:

It greatly harms performance on Linux tmpfs, 0.3s in my 5.2s link.

Indeed, and I'm fine with it being removed.

OK. Thanks! Then I will land this soon.

[MaskRay]

Update description that posix_fallocate is more harmful on FreeBSD UFS: "devastatingly slow on freebsd because UFS on freebsd does not have preallocation support like illumos. It zero-fills."

llvm/cmake/config-ix.cmake

 check_symbol_exists(_Unwind_Backtrace "unwind.h" HAVE__UNWIND_BACKTRACE)
 check_symbol_exists(getpagesize unistd.h HAVE_GETPAGESIZE)
 check_symbol_exists(sysconf unistd.h HAVE_SYSCONF)
 check_symbol_exists(getrusage sys/resource.h HAVE_GETRUSAGE)
 check_symbol_exists(setrlimit sys/resource.h HAVE_SETRLIMIT)
 check_symbol_exists(isatty unistd.h HAVE_ISATTY)
 check_symbol_exists(futimens sys/stat.h HAVE_FUTIMENS)
 check_symbol_exists(futimes sys/time.h HAVE_FUTIMES)
-check_symbol_exists(posix_fallocate fcntl.h HAVE_POSIX_FALLOCATE)
 # AddressSanitizer conflicts with lib/Support/Unix/Signals.inc
 # Avoid sigaltstack on Apple platforms, where backtrace() cannot handle it
 # (rdar://7089625) and _Unwind_Backtrace is unusable because it cannot unwind
 # past the signal handler after an assertion failure (rdar://29866587).
 if( HAVE_SIGNAL_H AND NOT LLVM_USE_SANITIZER MATCHES ".*Address.*" AND NOT APPLE )
   check_symbol_exists(sigaltstack signal.h HAVE_SIGALTSTACK)
 endif()
 set(CMAKE_REQUIRED_DEFINITIONS "-D_LARGEFILE64_SOURCE")

llvm/include/llvm/Config/config.h.cmake

 #cmakedefine HAVE_MALLINFO2 ${HAVE_MALLINFO2}
 
 /* Define to 1 if you have the <malloc/malloc.h> header file. */
 #cmakedefine HAVE_MALLOC_MALLOC_H ${HAVE_MALLOC_MALLOC_H}
 
 /* Define to 1 if you have the `malloc_zone_statistics' function. */
 #cmakedefine HAVE_MALLOC_ZONE_STATISTICS ${HAVE_MALLOC_ZONE_STATISTICS}
 
-/* Define to 1 if you have the `posix_fallocate' function. */
-#cmakedefine HAVE_POSIX_FALLOCATE ${HAVE_POSIX_FALLOCATE}
-
 /* Define to 1 if you have the `posix_spawn' function. */
 #cmakedefine HAVE_POSIX_SPAWN ${HAVE_POSIX_SPAWN}
 
 /* Define to 1 if you have the `pread' function. */
 #cmakedefine HAVE_PREAD ${HAVE_PREAD}
 
 /* Have pthread_getspecific */
 #cmakedefine HAVE_PTHREAD_GETSPECIFIC ${HAVE_PTHREAD_GETSPECIFIC}

llvm/lib/Support/Unix/Path.inc

std::error_code rename(const Twine &from, const Twine &to) {
 
   if (::rename(f.begin(), t.begin()) == -1)
     return std::error_code(errno, std::generic_category());
 
   return std::error_code();
 }
 
 std::error_code resize_file(int FD, uint64_t Size) {
-#if defined(HAVE_POSIX_FALLOCATE)
-  // If we have posix_fallocate use it. Unlike ftruncate it always allocates
-  // space, so we get an error if the disk is full.
-  if (int Err = ::posix_fallocate(FD, 0, Size)) {
-#ifdef _AIX
-    constexpr int NotSupportedError = ENOTSUP;
-#else
-    constexpr int NotSupportedError = EOPNOTSUPP;
-#endif
-    if (Err != EINVAL && Err != NotSupportedError)
-      return std::error_code(Err, std::generic_category());
-  }
-#endif
   // Use ftruncate as a fallback. It may or may not allocate space. At least on
   // OS X with HFS+ it does.
   if (::ftruncate(FD, Size) == -1)
     return std::error_code(errno, std::generic_category());
 
   return std::error_code();
 }
 

llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn

write_cmake_config("config") {
   ]
 
   if (current_os == "linux" || current_os == "android") {
     values += [
       "HAVE_FUTIMENS=1",
       "HAVE_LINK_H=1",
       "HAVE_LSEEK64=1",
       "HAVE_MALLINFO=1",
-      "HAVE_POSIX_FALLOCATE=1",
       "HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC=1",
     ]
   } else {
     values += [
       "HAVE_FUTIMENS=",
       "HAVE_LINK_H=",
       "HAVE_LSEEK64=",
       "HAVE_MALLINFO=",
-      "HAVE_POSIX_FALLOCATE=",
       "HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC=",
     ]
   }
 
   if (current_os == "mac") {
     values += [
       "HAVE_CRASHREPORTER_INFO=1",
       "HAVE_DECL_ARC4RANDOM=1",

utils/bazel/llvm-project-overlay/llvm/config.bzl

posix_defines = [
     "HAVE_UNISTD_H=1",
 ]
 
 linux_defines = posix_defines + [
     "_GNU_SOURCE",
     "HAVE_LINK_H=1",
     "HAVE_LSEEK64=1",
     "HAVE_MALLINFO=1",
-    "HAVE_POSIX_FALLOCATE=1",
     "HAVE_SBRK=1",
     "HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC=1",
 ]
 
 macos_defines = posix_defines + [
     "HAVE_MACH_MACH_H=1",
     "HAVE_MALLOC_MALLOC_H=1",
     "HAVE_MALLOC_ZONE_STATISTICS=1",

utils/bazel/llvm-project-overlay/llvm/include/llvm/Config/config.h

 /* HAVE_MALLINFO defined in Bazel */
 
 /* Define to 1 if you have the <malloc/malloc.h> header file. */
 /* HAVE_MALLOC_MALLOC_H defined in Bazel */
 
 /* Define to 1 if you have the `malloc_zone_statistics' function. */
 /* HAVE_MALLOC_ZONE_STATISTICS defined in Bazel */
 
-/* Define to 1 if you have the `posix_fallocate' function. */
-/* HAVE_POSIX_FALLOCATE defined in Bazel */
-
 /* Define to 1 if you have the `posix_spawn' function. */
 #define HAVE_POSIX_SPAWN 1
 
 /* Define to 1 if you have the `pread' function. */
 #define HAVE_PREAD 1
 
 /* Have pthread_getspecific */
 #define HAVE_PTHREAD_GETSPECIFIC 1

utils/bazel/llvm_configs/config.h.cmake

 #cmakedefine HAVE_MALLINFO2 ${HAVE_MALLINFO2}
 
 /* Define to 1 if you have the <malloc/malloc.h> header file. */
 #cmakedefine HAVE_MALLOC_MALLOC_H ${HAVE_MALLOC_MALLOC_H}
 
 /* Define to 1 if you have the `malloc_zone_statistics' function. */
 #cmakedefine HAVE_MALLOC_ZONE_STATISTICS ${HAVE_MALLOC_ZONE_STATISTICS}
 
-/* Define to 1 if you have the `posix_fallocate' function. */
-#cmakedefine HAVE_POSIX_FALLOCATE ${HAVE_POSIX_FALLOCATE}
-
 /* Define to 1 if you have the `posix_spawn' function. */
 #cmakedefine HAVE_POSIX_SPAWN ${HAVE_POSIX_SPAWN}
 
 /* Define to 1 if you have the `pread' function. */
 #cmakedefine HAVE_PREAD ${HAVE_PREAD}
 
 /* Have pthread_getspecific */
 #cmakedefine HAVE_PTHREAD_GETSPECIFIC ${HAVE_PTHREAD_GETSPECIFIC}