[llvm] r336276 - [ThinLTO] Update ThinLTO cache file atimes when on Windows

Andrew Ng via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 4 07:17:10 PDT 2018


Author: anng
Date: Wed Jul  4 07:17:10 2018
New Revision: 336276

URL: http://llvm.org/viewvc/llvm-project?rev=336276&view=rev
Log:
[ThinLTO] Update ThinLTO cache file atimes when on Windows

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

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

Modified: llvm/trunk/include/llvm/Support/FileSystem.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FileSystem.h?rev=336276&r1=336275&r2=336276&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/FileSystem.h (original)
+++ llvm/trunk/include/llvm/Support/FileSystem.h Wed Jul  4 07:17:10 2018
@@ -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.

Modified: llvm/trunk/lib/LTO/Caching.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/Caching.cpp?rev=336276&r1=336275&r2=336276&view=diff
==============================================================================
--- llvm/trunk/lib/LTO/Caching.cpp (original)
+++ llvm/trunk/lib/LTO/Caching.cpp Wed Jul  4 07:17:10 2018
@@ -19,6 +19,12 @@
 #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::localCa
     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::localCa
     // 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.

Modified: llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp?rev=336276&r1=336275&r2=336276&view=diff
==============================================================================
--- llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp (original)
+++ llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp Wed Jul  4 07:17:10 2018
@@ -55,6 +55,12 @@
 
 #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

Modified: llvm/trunk/lib/Support/Windows/Path.inc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Path.inc?rev=336276&r1=336275&r2=336276&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Windows/Path.inc (original)
+++ llvm/trunk/lib/Support/Windows/Path.inc Wed Jul  4 07:17:10 2018
@@ -1049,6 +1049,8 @@ static DWORD nativeAccess(FileAccess Acc
     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 Tw
       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);

Modified: llvm/trunk/test/ThinLTO/X86/cache.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/cache.ll?rev=336276&r1=336275&r2=336276&view=diff
==============================================================================
--- llvm/trunk/test/ThinLTO/X86/cache.ll (original)
+++ llvm/trunk/test/ThinLTO/X86/cache.ll Wed Jul  4 07:17:10 2018
@@ -80,6 +80,27 @@
 ; 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

Modified: llvm/trunk/unittests/Support/Path.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/Path.cpp?rev=336276&r1=336275&r2=336276&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/Path.cpp (original)
+++ llvm/trunk/unittests/Support/Path.cpp Wed Jul  4 07:17:10 2018
@@ -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,




More information about the llvm-commits mailing list