Skip to content

Commit

Permalink
FileManager: Shrink FileEntryRef to the size of a pointer
Browse files Browse the repository at this point in the history 
Shrink `FileEntryRef` to the size of a pointer, by having it directly
reference the `StringMapEntry` the same way that `DirectoryEntryRef`
does. This makes `FileEntryRef::FileEntryRef` private as a side effect
(`FileManager` is a friend!).

There are two helper types added within `FileEntryRef`:

- `FileEntryRef::MapValue` is the type stored in
  `FileManager::SeenFileEntries`. It's a replacement for
  `SeenFileEntryOrRedirect`, where the second pointer type has been
  changed from `StringRef*` to `MapEntry*` (see next bullet).
- `FileEntryRef::MapEntry` is the instantiation of `StringMapEntry<>`
  where `MapValue` is stored. This is what `FileEntryRef` has a pointer
  to, in order to grab the name in addition to the value.

Differential Revision: https://reviews.llvm.org/D89488
  • Loading branch information
@dexonsmith
dexonsmith committed Oct 27, 2020
1 parent 83154c5 commit 917acac
Show file tree
Hide file tree
Showing 4 changed files with 148 additions and 81 deletions.
78 changes: 54 additions & 24 deletions clang/include/clang/Basic/FileManager.h
View file
Expand Up @@ -77,12 +77,10 @@ class FileEntry;
/// accessed by the FileManager's client.
class FileEntryRef {
public:
FileEntryRef() = delete;
FileEntryRef(StringRef Name, const FileEntry &Entry)
: Name(Name), Entry(&Entry) {}

const StringRef getName() const { return Name; }
const FileEntry &getFileEntry() const { return *Entry; }
const StringRef getName() const { return Entry->first(); }
const FileEntry &getFileEntry() const {
return *Entry->second->V.get<FileEntry *>();
}

inline bool isValid() const;
inline off_t getSize() const;
Expand All @@ -91,15 +89,43 @@ class FileEntryRef {
inline time_t getModificationTime() const;

friend bool operator==(const FileEntryRef &LHS, const FileEntryRef &RHS) {
return LHS.Entry == RHS.Entry && LHS.Name == RHS.Name;
return LHS.Entry == RHS.Entry;
}
friend bool operator!=(const FileEntryRef &LHS, const FileEntryRef &RHS) {
return !(LHS == RHS);
}

struct MapValue;

/// Type used in the StringMap.
using MapEntry = llvm::StringMapEntry<llvm::ErrorOr<MapValue>>;

/// Type stored in the StringMap.
struct MapValue {
/// The pointer at another MapEntry is used when the FileManager should
/// silently forward from one name to another, which occurs in Redirecting
/// VFSs that use external names. In that case, the \c FileEntryRef
/// returned by the \c FileManager will have the external name, and not the
/// name that was used to lookup the file.
llvm::PointerUnion<FileEntry *, const MapEntry *> V;

MapValue() = delete;
MapValue(FileEntry &FE) : V(&FE) {}
MapValue(MapEntry &ME) : V(&ME) {}
};

private:
StringRef Name;
const FileEntry *Entry;
friend class FileManager;

FileEntryRef() = delete;
explicit FileEntryRef(const MapEntry &Entry)
: Entry(&Entry) {
assert(Entry.second && "Expected payload");
assert(Entry.second->V && "Expected non-null");
assert(Entry.second->V.is<FileEntry *>() && "Expected FileEntry");
}

const MapEntry *Entry;
};

/// Cached information about one file (either on disk
Expand All @@ -110,7 +136,6 @@ class FileEntryRef {
class FileEntry {
friend class FileManager;

StringRef Name; // Name of the file.
std::string RealPathName; // Real path to the file; could be empty.
off_t Size; // File size in bytes.
time_t ModTime; // Modification time of file.
Expand All @@ -123,6 +148,14 @@ class FileEntry {
/// The open file, if it is owned by the \p FileEntry.
mutable std::unique_ptr<llvm::vfs::File> File;

// First access name for this FileEntry.
//
// This is Optional only to allow delayed construction (FileEntryRef has no
// default constructor). It should always have a value in practice.
//
// TODO: remote this once everyone that needs a name uses FileEntryRef.
Optional<FileEntryRef> LastRef;

public:
FileEntry()
: UniqueID(0, 0), IsNamedPipe(false), IsValid(false)
Expand All @@ -131,7 +164,9 @@ class FileEntry {
FileEntry(const FileEntry &) = delete;
FileEntry &operator=(const FileEntry &) = delete;

StringRef getName() const { return Name; }
StringRef getName() const { return LastRef->getName(); }
FileEntryRef getLastRef() const { return *LastRef; }

StringRef tryGetRealPathName() const { return RealPathName; }
bool isValid() const { return IsValid; }
off_t getSize() const { return Size; }
Expand Down Expand Up @@ -212,26 +247,21 @@ class FileManager : public RefCountedBase<FileManager> {
llvm::StringMap<llvm::ErrorOr<DirectoryEntry &>, llvm::BumpPtrAllocator>
SeenDirEntries;

/// A reference to the file entry that is associated with a particular
/// filename, or a reference to another filename that should be looked up
/// instead of the accessed filename.
///
/// The reference to another filename is specifically useful for Redirecting
/// VFSs that use external names. In that case, the \c FileEntryRef returned
/// by the \c FileManager will have the external name, and not the name that
/// was used to lookup the file.
using SeenFileEntryOrRedirect =
llvm::PointerUnion<FileEntry *, const StringRef *>;

/// A cache that maps paths to file entries (either real or
/// virtual) we have looked up, or an error that occurred when we looked up
/// the file.
///
/// \see SeenDirEntries
llvm::StringMap<llvm::ErrorOr<SeenFileEntryOrRedirect>,
llvm::BumpPtrAllocator>
llvm::StringMap<llvm::ErrorOr<FileEntryRef::MapValue>, llvm::BumpPtrAllocator>
SeenFileEntries;

/// A mirror of SeenFileEntries to give fake answers for getBypassFile().
///
/// Don't bother hooking up a BumpPtrAllocator. This should be rarely used,
/// and only on error paths.
std::unique_ptr<llvm::StringMap<llvm::ErrorOr<FileEntryRef::MapValue>>>
SeenBypassFileEntries;

/// The canonical names of files and directories .
llvm::DenseMap<const void *, llvm::StringRef> CanonicalNames;

Expand Down
106 changes: 62 additions & 44 deletions clang/lib/Basic/FileManager.cpp
View file
Expand Up @@ -212,11 +212,10 @@ FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) {
SeenFileInsertResult.first->second.getError());
// Construct and return and FileEntryRef, unless it's a redirect to another
// filename.
SeenFileEntryOrRedirect Value = *SeenFileInsertResult.first->second;
FileEntry *FE;
if (LLVM_LIKELY(FE = Value.dyn_cast<FileEntry *>()))
return FileEntryRef(SeenFileInsertResult.first->first(), *FE);
return getFileRef(*Value.get<const StringRef *>(), openFile, CacheFailure);
FileEntryRef::MapValue Value = *SeenFileInsertResult.first->second;
if (LLVM_LIKELY(Value.V.is<FileEntry *>()))
return FileEntryRef(*SeenFileInsertResult.first);
return FileEntryRef(*Value.V.get<const FileEntryRef::MapEntry *>());
}

// We've not seen this before. Fill it in.
Expand Down Expand Up @@ -268,26 +267,29 @@ FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) {
// This occurs when one dir is symlinked to another, for example.
FileEntry &UFE = UniqueRealFiles[Status.getUniqueID()];

NamedFileEnt->second = &UFE;

// If the name returned by getStatValue is different than Filename, re-intern
// the name.
if (Status.getName() != Filename) {
auto &NewNamedFileEnt =
*SeenFileEntries.insert({Status.getName(), &UFE}).first;
assert((*NewNamedFileEnt.second).get<FileEntry *>() == &UFE &&
if (Status.getName() == Filename) {
// The name matches. Set the FileEntry.
NamedFileEnt->second = FileEntryRef::MapValue(UFE);
} else {
// Name mismatch. We need a redirect. First grab the actual entry we want
// to return.
auto &Redirection =
*SeenFileEntries.insert({Status.getName(), FileEntryRef::MapValue(UFE)})
.first;
assert(Redirection.second->V.is<FileEntry *>() &&
"filename redirected to a non-canonical filename?");
assert(Redirection.second->V.get<FileEntry *>() == &UFE &&
"filename from getStatValue() refers to wrong file");
InterndFileName = NewNamedFileEnt.first().data();
// In addition to re-interning the name, construct a redirecting seen file
// entry, that will point to the name the filesystem actually wants to use.
StringRef *Redirect = new (CanonicalNameStorage) StringRef(InterndFileName);
auto SeenFileInsertResultIt = SeenFileEntries.find(Filename);
assert(SeenFileInsertResultIt != SeenFileEntries.end() &&
"unexpected SeenFileEntries cache miss");
SeenFileInsertResultIt->second = Redirect;
NamedFileEnt = &*SeenFileInsertResultIt;

// Cache the redirection in the previously-inserted entry, still available
// in the tentative return value.
NamedFileEnt->second = FileEntryRef::MapValue(Redirection);

// Fix the tentative return value.
NamedFileEnt = &Redirection;
}

FileEntryRef ReturnedRef(*NamedFileEnt);
if (UFE.isValid()) { // Already have an entry with this inode, return it.

// FIXME: this hack ensures that if we look up a file by a virtual path in
Expand All @@ -299,20 +301,20 @@ FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) {
if (DirInfo != UFE.Dir && Status.IsVFSMapped)
UFE.Dir = DirInfo;

// Always update the name to use the last name by which a file was accessed.
// FIXME: Neither this nor always using the first name is correct; we want
// to switch towards a design where we return a FileName object that
// Always update LastRef to the last name by which a file was accessed.
// FIXME: Neither this nor always using the first reference is correct; we
// want to switch towards a design where we return a FileName object that
// encapsulates both the name by which the file was accessed and the
// corresponding FileEntry.
// FIXME: The Name should be removed from FileEntry once all clients
// adopt FileEntryRef.
UFE.Name = InterndFileName;
// FIXME: LastRef should be removed from FileEntry once all clients adopt
// FileEntryRef.
UFE.LastRef = ReturnedRef;

return FileEntryRef(InterndFileName, UFE);
return ReturnedRef;
}

// Otherwise, we don't have this file yet, add it.
UFE.Name = InterndFileName;
UFE.LastRef = ReturnedRef;
UFE.Size = Status.getSize();
UFE.ModTime = llvm::sys::toTimeT(Status.getLastModificationTime());
UFE.Dir = DirInfo;
Expand All @@ -329,7 +331,7 @@ FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) {
// We should still fill the path even if we aren't opening the file.
fillRealPathName(&UFE, InterndFileName);
}
return FileEntryRef(InterndFileName, UFE);
return ReturnedRef;
}

const FileEntry *
Expand All @@ -341,12 +343,12 @@ FileManager::getVirtualFile(StringRef Filename, off_t Size,
auto &NamedFileEnt = *SeenFileEntries.insert(
{Filename, std::errc::no_such_file_or_directory}).first;
if (NamedFileEnt.second) {
SeenFileEntryOrRedirect Value = *NamedFileEnt.second;
FileEntryRef::MapValue Value = *NamedFileEnt.second;
FileEntry *FE;
if (LLVM_LIKELY(FE = Value.dyn_cast<FileEntry *>()))
if (LLVM_LIKELY(FE = Value.V.dyn_cast<FileEntry *>()))
return FE;
return getVirtualFile(*Value.get<const StringRef *>(), Size,
ModificationTime);
return &FileEntryRef(*Value.V.get<const FileEntryRef::MapEntry *>())
.getFileEntry();
}

// We've not seen this before, or the file is cached as non-existent.
Expand All @@ -372,7 +374,7 @@ FileManager::getVirtualFile(StringRef Filename, off_t Size,
Status.getUser(), Status.getGroup(), Size,
Status.getType(), Status.getPermissions());

NamedFileEnt.second = UFE;
NamedFileEnt.second = FileEntryRef::MapValue(*UFE);

// If we had already opened this file, close it now so we don't
// leak the descriptor. We're not going to use the file
Expand All @@ -381,6 +383,9 @@ FileManager::getVirtualFile(StringRef Filename, off_t Size,
UFE->closeFile();

// If we already have an entry with this inode, return it.
//
// FIXME: Surely this should add a reference by the new name, and return
// it instead...
if (UFE->isValid())
return UFE;

Expand All @@ -390,10 +395,10 @@ FileManager::getVirtualFile(StringRef Filename, off_t Size,
} else {
VirtualFileEntries.push_back(std::make_unique<FileEntry>());
UFE = VirtualFileEntries.back().get();
NamedFileEnt.second = UFE;
NamedFileEnt.second = FileEntryRef::MapValue(*UFE);
}

UFE->Name = InterndFileName;
UFE->LastRef = FileEntryRef(NamedFileEnt);
UFE->Size = Size;
UFE->ModTime = ModificationTime;
UFE->Dir = *DirInfo;
Expand All @@ -409,17 +414,30 @@ llvm::Optional<FileEntryRef> FileManager::getBypassFile(FileEntryRef VF) {
if (getStatValue(VF.getName(), Status, /*isFile=*/true, /*F=*/nullptr))
return None;

// Fill it in from the stat.
if (!SeenBypassFileEntries)
SeenBypassFileEntries = std::make_unique<
llvm::StringMap<llvm::ErrorOr<FileEntryRef::MapValue>>>();

// If we've already bypassed just use the existing one.
auto Insertion = SeenBypassFileEntries->insert(
{VF.getName(), std::errc::no_such_file_or_directory});
if (!Insertion.second)
return FileEntryRef(*Insertion.first);

// Fill in the new entry from the stat.
BypassFileEntries.push_back(std::make_unique<FileEntry>());
const FileEntry &VFE = VF.getFileEntry();
FileEntry &BFE = *BypassFileEntries.back();
BFE.Name = VFE.getName();
Insertion.first->second = FileEntryRef::MapValue(BFE);
BFE.LastRef = FileEntryRef(*Insertion.first);
BFE.Size = Status.getSize();
BFE.Dir = VFE.Dir;
BFE.ModTime = llvm::sys::toTimeT(Status.getLastModificationTime());
BFE.UID = NextFileUID++;
BFE.IsValid = true;
return FileEntryRef(VF.getName(), BFE);

// Save the entry in the bypass table and return.
return FileEntryRef(*Insertion.first);
}

bool FileManager::FixupRelativePath(SmallVectorImpl<char> &path) const {
Expand Down Expand Up @@ -534,13 +552,13 @@ void FileManager::GetUniqueIDMapping(
UIDToFiles.resize(NextFileUID);

// Map file entries
for (llvm::StringMap<llvm::ErrorOr<SeenFileEntryOrRedirect>,
for (llvm::StringMap<llvm::ErrorOr<FileEntryRef::MapValue>,
llvm::BumpPtrAllocator>::const_iterator
FE = SeenFileEntries.begin(),
FEEnd = SeenFileEntries.end();
FE != FEEnd; ++FE)
if (llvm::ErrorOr<SeenFileEntryOrRedirect> Entry = FE->getValue()) {
if (const auto *FE = (*Entry).dyn_cast<FileEntry *>())
if (llvm::ErrorOr<FileEntryRef::MapValue> Entry = FE->getValue()) {
if (const auto *FE = Entry->V.dyn_cast<FileEntry *>())
UIDToFiles[FE->getUID()] = FE;
}

Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Basic/SourceManager.cpp
View file
Expand Up @@ -702,7 +702,7 @@ const FileEntry *
SourceManager::bypassFileContentsOverride(const FileEntry &File) {
assert(isFileOverridden(&File));
llvm::Optional<FileEntryRef> BypassFile =
FileMgr.getBypassFile(FileEntryRef(File.getName(), File));
FileMgr.getBypassFile(File.getLastRef());

// If the file can't be found in the FS, give up.
if (!BypassFile)
Expand Down

0 comments on commit 917acac

Please sign in to comment.