Skip to content

Commit

Permalink
[ThinLTO] Update ThinLTO cache file atimes when on Windows
Browse files Browse the repository at this point in the history
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
  • Loading branch information
nga888 committed Jul 4, 2018
1 parent 67676e9 commit 089303d
Show file tree
Hide file tree
Showing 6 changed files with 112 additions and 9 deletions.
3 changes: 3 additions & 0 deletions llvm/include/llvm/Support/FileSystem.h
Expand Up @@ -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.
Expand Down
31 changes: 23 additions & 8 deletions llvm/lib/LTO/Caching.cpp
Expand Up @@ -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;

Expand All @@ -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
Expand All @@ -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.
Expand Down
19 changes: 18 additions & 1 deletion llvm/lib/LTO/ThinLTOCodeGenerator.cpp
Expand Up @@ -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"
Expand Down Expand Up @@ -391,7 +397,18 @@ class ModuleCacheEntry {
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
Expand Down
15 changes: 15 additions & 0 deletions llvm/lib/Support/Windows/Path.inc
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
Expand Down
21 changes: 21 additions & 0 deletions llvm/test/ThinLTO/X86/cache.ll
Expand Up @@ -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
Expand Down
32 changes: 32 additions & 0 deletions llvm/unittests/Support/Path.cpp
Expand Up @@ -26,6 +26,7 @@

#ifdef _WIN32
#include "llvm/ADT/ArrayRef.h"
#include "llvm/Support/Chrono.h"
#include <windows.h>
#include <winerror.h>
#endif
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 089303d

Please sign in to comment.