[ThinLTO] Update ThinLTO cache file atimes when on Windows
authorAndrew Ng <anng.sw@gmail.com>
Wed, 4 Jul 2018 14:17:10 +0000 (14:17 +0000)
committerAndrew Ng <anng.sw@gmail.com>
Wed, 4 Jul 2018 14:17:10 +0000 (14:17 +0000)
ThinLTO cache file access times are used for expiration based pruning
and since Vista, file access times are not updated by Windows by
default:

https://blogs.technet.microsoft.com/filecab/2006/11/07/disabling-last-access-time-in-windows-vista-to-improve-ntfs-performance

This means on Windows, cache files are currently being pruned from
creation time. This change manually updates cache files that are
accessed by ThinLTO, when on Windows.

Patch by Owen Reynolds.

Differential Revision: https://reviews.llvm.org/D47266

llvm-svn: 336276

llvm/include/llvm/Support/FileSystem.h
llvm/lib/LTO/Caching.cpp
llvm/lib/LTO/ThinLTOCodeGenerator.cpp
llvm/lib/Support/Windows/Path.inc
llvm/test/ThinLTO/X86/cache.ll
llvm/unittests/Support/Path.cpp

index 4b42196..02db459 100644 (file)
@@ -728,6 +728,9 @@ enum OpenFlags : unsigned {
   /// When a child process is launched, this file should remain open in the
   /// child process.
   OF_ChildInherit = 8,
+
+  /// Force files Atime to be updated on access. Only makes a difference on windows.
+  OF_UpdateAtime = 16,
 };
 
 /// Create a uniquely named file.
index bd6190d..089e77e 100644 (file)
 #include "llvm/Support/Process.h"
 #include "llvm/Support/raw_ostream.h"
 
+#if !defined(_MSC_VER) && !defined(__MINGW32__)
+#include <unistd.h>
+#else
+#include <io.h>
+#endif
+
 using namespace llvm;
 using namespace llvm::lto;
 
@@ -33,11 +39,21 @@ Expected<NativeObjectCache> lto::localCache(StringRef CacheDirectoryPath,
     SmallString<64> EntryPath;
     sys::path::append(EntryPath, CacheDirectoryPath, "llvmcache-" + Key);
     // First, see if we have a cache hit.
-    ErrorOr<std::unique_ptr<MemoryBuffer>> MBOrErr =
-        MemoryBuffer::getFile(EntryPath);
-    if (MBOrErr) {
-      AddBuffer(Task, std::move(*MBOrErr));
-      return AddStreamFn();
+    int FD;
+    SmallString<64> ResultPath;
+    std::error_code EC = sys::fs::openFileForRead(
+        Twine(EntryPath), FD, sys::fs::OF_UpdateAtime, &ResultPath);
+    if (!EC) {
+      ErrorOr<std::unique_ptr<MemoryBuffer>> MBOrErr =
+          MemoryBuffer::getOpenFile(FD, EntryPath,
+                                    /*FileSize*/ -1,
+                                    /*RequiresNullTerminator*/ false);
+      close(FD);
+      if (MBOrErr) {
+        AddBuffer(Task, std::move(*MBOrErr));
+        return AddStreamFn();
+      }
+      EC = MBOrErr.getError();
     }
 
     // On Windows we can fail to open a cache file with a permission denied
@@ -46,10 +62,9 @@ Expected<NativeObjectCache> lto::localCache(StringRef CacheDirectoryPath,
     // process has opened the file without the sharing permissions we need.
     // Since the file is probably being deleted we handle it in the same way as
     // if the file did not exist at all.
-    if (MBOrErr.getError() != errc::no_such_file_or_directory &&
-        MBOrErr.getError() != errc::permission_denied)
+    if (EC != errc::no_such_file_or_directory && EC != errc::permission_denied)
       report_fatal_error(Twine("Failed to open cache file ") + EntryPath +
-                         ": " + MBOrErr.getError().message() + "\n");
+                         ": " + EC.message() + "\n");
 
     // This native object stream is responsible for commiting the resulting
     // file to the cache and calling AddBuffer to add it to the link.
index 8c65a86..d7fac6a 100644 (file)
 
 #include <numeric>
 
+#if !defined(_MSC_VER) && !defined(__MINGW32__)
+#include <unistd.h>
+#else
+#include <io.h>
+#endif
+
 using namespace llvm;
 
 #define DEBUG_TYPE "thinlto"
@@ -391,7 +397,18 @@ public:
   ErrorOr<std::unique_ptr<MemoryBuffer>> tryLoadingBuffer() {
     if (EntryPath.empty())
       return std::error_code();
-    return MemoryBuffer::getFile(EntryPath);
+    int FD;
+    SmallString<64> ResultPath;
+    std::error_code EC = sys::fs::openFileForRead(
+        Twine(EntryPath), FD, sys::fs::OF_UpdateAtime, &ResultPath);
+    if (EC)
+      return EC;
+    ErrorOr<std::unique_ptr<MemoryBuffer>> MBOrErr =
+        MemoryBuffer::getOpenFile(FD, EntryPath,
+                                  /*FileSize*/ -1,
+                                  /*RequiresNullTerminator*/ false);
+    close(FD);
+    return MBOrErr;
   }
 
   // Cache the Produced object file
index 577d571..f425d60 100644 (file)
@@ -1049,6 +1049,8 @@ static DWORD nativeAccess(FileAccess Access, OpenFlags Flags) {
     Result |= GENERIC_WRITE;
   if (Flags & OF_Delete)
     Result |= DELETE;
+  if (Flags & OF_UpdateAtime)
+    Result |= FILE_WRITE_ATTRIBUTES;
   return Result;
 }
 
@@ -1104,6 +1106,19 @@ Expected<file_t> openNativeFile(const Twine &Name, CreationDisposition Disp,
       Name, Result, NativeDisp, NativeAccess, FILE_ATTRIBUTE_NORMAL, Inherit);
   if (EC)
     return errorCodeToError(EC);
+
+  if (Flags & OF_UpdateAtime) {
+    FILETIME FileTime;
+    SYSTEMTIME SystemTime;
+    GetSystemTime(&SystemTime);
+    if (SystemTimeToFileTime(&SystemTime, &FileTime) == 0 ||
+        SetFileTime(Result, NULL, &FileTime, NULL) == 0) {
+      DWORD LastError = ::GetLastError();
+      ::CloseHandle(Result);
+      return errorCodeToError(mapWindowsError(LastError));
+    }
+  }
+
   if (Flags & OF_Delete) {
     if ((EC = setDeleteDisposition(Result, true))) {
       ::CloseHandle(Result);
index 6fddfb6..bcd4f09 100644 (file)
 ; RUN: llvm-lto -thinlto-action=run -exported-symbol=globalfunc %t2.bc %t.bc -thinlto-cache-dir %t.cache --thinlto-cache-pruning-interval 0
 ; RUN: not ls %t.cache/llvmcache-foo
 
+; Populate the cache with files with "old" access times, then check llvm-lto updates these file times
+; A negative pruning interval is used to avoid removing cache entries
+; RUN: rm -Rf %t.cache && mkdir %t.cache
+; RUN: llvm-lto -thinlto-action=run -exported-symbol=globalfunc %t2.bc %t.bc -thinlto-cache-dir %t.cache
+; RUN: touch -a -t 197001011200 %t.cache/llvmcache-*
+; RUN: llvm-lto -thinlto-action=run -exported-symbol=globalfunc %t2.bc %t.bc -thinlto-cache-dir %t.cache --thinlto-cache-pruning-interval -1
+; RUN: ls -ltu %t.cache/* | not grep 1970-01-01
+
+; Populate the cache with files with "old" access times, then check llvm-lto2 updates these file times
+; RUN: rm -Rf %t.cache
+; RUN: llvm-lto2 run -o %t.o %t2.bc %t.bc -cache-dir %t.cache \
+; RUN:  -r=%t2.bc,_main,plx \
+; RUN:  -r=%t2.bc,_globalfunc,lx \
+; RUN:  -r=%t.bc,_globalfunc,plx
+; RUN: touch -a -t 197001011200 %t.cache/llvmcache-*
+; RUN: llvm-lto2 run -o %t.o %t2.bc %t.bc -cache-dir %t.cache \
+; RUN:  -r=%t2.bc,_main,plx \
+; RUN:  -r=%t2.bc,_globalfunc,lx \
+; RUN:  -r=%t.bc,_globalfunc,plx
+; RUN: ls -ltu %t.cache/* | not grep 1970-01-01
+
 ; Verify that specifying max size for the cache directory prunes it to this
 ; size, removing the largest files first.
 ; RUN: rm -Rf %t.cache && mkdir %t.cache
index ff301dc..dcbe94c 100644 (file)
@@ -26,6 +26,7 @@
 
 #ifdef _WIN32
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/Chrono.h"
 #include <windows.h>
 #include <winerror.h>
 #endif
@@ -1251,8 +1252,39 @@ TEST_F(FileSystemTest, OpenFileForRead) {
     ASSERT_NO_ERROR(fs::getUniqueID(Twine(ResultPath), D2));
     ASSERT_EQ(D1, D2);
   }
+  ::close(FileDescriptor);
+  ::close(FileDescriptor2);
+
+#ifdef _WIN32
+  // Since Windows Vista, file access time is not updated by default.
+  // This is instead updated manually by openFileForRead.
+  // https://blogs.technet.microsoft.com/filecab/2006/11/07/disabling-last-access-time-in-windows-vista-to-improve-ntfs-performance/
+  // This part of the unit test is Windows specific as the updating of
+  // access times can be disabled on Linux using /etc/fstab.
+
+  // Set access time to UNIX epoch.
+  ASSERT_NO_ERROR(sys::fs::openFileForWrite(Twine(TempPath), FileDescriptor,
+                                            fs::CD_OpenExisting));
+  TimePoint<> Epoch(std::chrono::milliseconds(0));
+  ASSERT_NO_ERROR(fs::setLastModificationAndAccessTime(FileDescriptor, Epoch));
+  ::close(FileDescriptor);
 
+  // Open the file and ensure access time is updated, when forced.
+  bool ForceATime = true;
+  ASSERT_NO_ERROR(fs::openFileForRead(Twine(TempPath), FileDescriptor,
+                                      fs::OF_UpdateAtime, &ResultPath));
+
+  sys::fs::file_status Status;
+  ASSERT_NO_ERROR(sys::fs::status(FileDescriptor, Status));
+  auto FileAccessTime = Status.getLastAccessedTime();
+
+  ASSERT_NE(Epoch, FileAccessTime);
   ::close(FileDescriptor);
+
+  // Ideally this test would include a case when ATime is not forced to update,
+  // however the expected behaviour will differ depending on the configuration
+  // of the Windows file system.
+#endif
 }
 
 static void createFileWithData(const Twine &Path, bool ShouldExistBefore,