SourceManager: Fix an SLocEntry memory regression introduced with FileEntryRef

[dexonsmith]

Summary

4dc5573acc0d2e7c59d8bac2543eb25cb4b32984 added FileEntryRef in order to
help enable sharing of a FileManager between CompilerInstances.

It also added a StringRef with the filename on FileInfo. This
doubled sizeof(FileInfo), bloating sizeof(SLocEntry), of which we
have one for each (loaded and unloaded) file and macro expansion. This
causes a memory regression in modules builds.

Move the filename down into the ContentCache, which is a side data
structure for FileInfo that does not impact sizeof(SLocEntry). Once
FileEntryRef is used for ContentCache::OrigEntry this can go away.

[JDevlieghere]

clang/include/clang/Basic/SourceManager.h

315

Would it possibly make more sense to drop the const qualifier from the argument?

[dexonsmith]

clang/include/clang/Basic/SourceManager.h

315

Yes, that's probably better. I'll make a prep commit that changes getOrCreateContentCache() to return non-const and thread that through to here, and then this is just Con.Filename = Filename.

[dexonsmith]

Rebased on top of https://reviews.llvm.org/D89580, which implements @JDevlieghere's suggestion.

[dexonsmith]

Thanks for the review! @JDevlieghere, let me know if the updated patch looks good to you too (also the prep commit).

clang/include/clang/Basic/SourceManager.h

namespace SrcMgr {
 
   public:
     /// Reference to the file entry representing this ContentCache.
     ///
     /// This reference does not own the FileEntry object.
     ///
     /// It is possible for this to be NULL if the ContentCache encapsulates
     /// an imaginary text buffer.
+    ///
+    /// FIXME: Turn this into a FileEntryRef and remove Filename.
     const FileEntry *OrigEntry;
 
     /// References the file which the contents were actually loaded from.
     ///
     /// Can be different from 'Entry' if we overridden the contents of one file
     /// with the contents of another file.
     const FileEntry *ContentsEntry;
 
+    /// The filename that is used to access OrigEntry.
+    ///
+    /// FIXME: Remove this once OrigEntry is a FileEntryRef with a stable name.
+    StringRef Filename;
+
     /// A bump pointer allocated array of offsets for each source line.
     ///
     /// This is lazily computed.  The lines are owned by the SourceManager
     /// BumpPointerAllocator object.
     mutable LineOffsetMapping SourceLineCache;
 
     /// Indicates whether the buffer itself was provided to override
     /// the actual file contents.
namespace SrcMgr {
 
   /// Information about a FileID, basically just the logical file
   /// that it represents and include stack information.
   ///
   /// Each FileInfo has include stack information, indicating where it came
   /// from. This information encodes the \#include chain that a token was
   /// expanded from. The main include file has an invalid IncludeLoc.
   ///
-  /// FileInfos contain a "ContentCache *", with the contents of the file.
+  /// FileInfo should not grow larger than ExpansionInfo. Doing so will
+  /// cause memory to bloat in compilations with many unloaded macro
+  /// expansions, since the two data structurs are stored in a union in
+  /// SLocEntry. Extra fields should instead go in "ContentCache *", which
+  /// stores file contents and other bits on the side.
   ///
   class FileInfo {
     friend class clang::SourceManager;
     friend class clang::ASTWriter;
     friend class clang::ASTReader;
 
     /// The location of the \#include that brought in this file.
     ///
     /// This is an invalid SLOC for the main file (top of the \#include chain).
     unsigned IncludeLoc;  // Really a SourceLocation
 
     /// Number of FileIDs (files and macros) that were created during
     /// preprocessing of this \#include, including this SLocEntry.
     ///
     /// Zero means the preprocessor didn't provide such info for this SLocEntry.
     unsigned NumCreatedFIDs : 31;
 
     /// Whether this FileInfo has any \#line directives.
     unsigned HasLineDirectives : 1;
 
     /// The content cache and the characteristic of the file.
     llvm::PointerIntPair<const ContentCache*, 3, CharacteristicKind>
         ContentAndKind;
 
-    /// The filename that is used to access the file entry represented by the
-    /// content cache.
-    StringRef Filename;
-
   public:
     /// Return a FileInfo object.
     static FileInfo get(SourceLocation IL, ContentCache &Con,
                         CharacteristicKind FileCharacter, StringRef Filename) {
       FileInfo X;
       X.IncludeLoc = IL.getRawEncoding();
       X.NumCreatedFIDs = 0;
       X.HasLineDirectives = false;
       X.ContentAndKind.setPointer(&Con);
       X.ContentAndKind.setInt(FileCharacter);
-      X.Filename = Filename;
+      Con.Filename = Filename;
       return X;
     }
 
     SourceLocation getIncludeLoc() const {
       return SourceLocation::getFromRawEncoding(IncludeLoc);
     }
 
     const ContentCache &getContentCache() const {
public:
     /// Set the flag that indicates that this FileID has
     /// line table entries associated with it.
     void setHasLineDirectives() {
       HasLineDirectives = true;
     }
 
     /// Returns the name of the file that was used when the file was loaded from
     /// the underlying file system.
-    StringRef getName() const { return Filename; }
+    StringRef getName() const { return getContentCache().Filename; }
   };
 
   /// Each ExpansionInfo encodes the expansion location - where
   /// the token was ultimately expanded, and the SpellingLoc - where the actual
   /// character data for the token came from.
   class ExpansionInfo {
     // Really these are all SourceLocations.
 
public:
     /// a token range.
     static ExpansionInfo createForTokenSplit(SourceLocation SpellingLoc,
                                              SourceLocation Start,
                                              SourceLocation End) {
       return create(SpellingLoc, Start, End, false);
     }
   };
 
+  // Assert that the \c FileInfo objects are no bigger than \c ExpansionInfo
+  // objects. This controls the size of \c SLocEntry, of which we have one for
+  // each macro expansion. The number of (unloaded) macro expansions can be
+  // very large. Any other fields needed in FileInfo should go in ContentCache.
+  static_assert(sizeof(FileInfo) <= sizeof(ExpansionInfo),
+                "FileInfo must be no larger than ExpansionInfo.");
+
   /// This is a discriminated union of FileInfo and ExpansionInfo.
   ///
   /// SourceManager keeps an array of these objects, and they are uniquely
   /// identified by the FileID datatype.
   class SLocEntry {
     unsigned Offset : 31;
     unsigned IsExpansion : 1;
     union {

clang/lib/Basic/SourceManager.cpp

if (Entry.isFile()) {
         continue;
 
       SourceLocation IncludeLoc = File.getIncludeLoc();
       bool IncludedInFID =
           (IncludeLoc.isValid() && isInFileID(IncludeLoc, FID)) ||
           // Predefined header doesn't have a valid include location in main
           // file, but any files created by it should still be skipped when
           // computing macro args expanded in the main file.
-          (FID == MainFileID && Entry.getFile().Filename == "<built-in>");
+          (FID == MainFileID && Entry.getFile().getName() == "<built-in>");
       if (IncludedInFID) {
         // Skip the files/macros of the #include'd file, we only care about
         // macros that lexed macro arguments from our file.
         if (Entry.getFile().NumCreatedFIDs)
           ID += Entry.getFile().NumCreatedFIDs - 1 /*because of next ++ID*/;
         continue;
       } else if (IncludeLoc.isValid()) {
         // If file was included but not from FID, there is no more files/macros