Skip to content

Commit e494e26

Browse files
jansvoboda11jansvoboda11
authored
[clang][lex] Remove HeaderFileInfo::Framework (#114460)
This PR removes the `HeaderFileInfo::Framework` member and reduces the size of this data type from 32B to 16B. This should improve Clang's memory usage in situations where it keeps track of lots of header files. NFCI. Depends on #114459.
1 parent a1987be commit e494e26

File tree

8 files changed

+38
-96
lines changed

8 files changed

+38
-96
lines changed

‎clang-tools-extra/clangd/index/SymbolCollector.cpp

Lines changed: 28 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -335,9 +335,10 @@ class SymbolCollector::HeaderFileURICache {
335335
}
336336

337337
struct FrameworkHeaderPath {
338-
// Path to the framework directory containing the Headers/PrivateHeaders
339-
// directories e.g. /Frameworks/Foundation.framework/
340-
llvm::StringRef HeadersParentDir;
338+
// Path to the frameworks directory containing the .framework directory.
339+
llvm::StringRef FrameworkParentDir;
340+
// Name of the framework.
341+
llvm::StringRef FrameworkName;
341342
// Subpath relative to the Headers or PrivateHeaders dir, e.g. NSObject.h
342343
// Note: This is NOT relative to the `HeadersParentDir`.
343344
llvm::StringRef HeaderSubpath;
@@ -351,19 +352,17 @@ class SymbolCollector::HeaderFileURICache {
351352
path::reverse_iterator I = path::rbegin(Path);
352353
path::reverse_iterator Prev = I;
353354
path::reverse_iterator E = path::rend(Path);
355+
FrameworkHeaderPath HeaderPath;
354356
while (I != E) {
355-
if (*I == "Headers") {
356-
FrameworkHeaderPath HeaderPath;
357-
HeaderPath.HeadersParentDir = Path.substr(0, I - E);
357+
if (*I == "Headers" || *I == "PrivateHeaders") {
358358
HeaderPath.HeaderSubpath = Path.substr(Prev - E);
359-
HeaderPath.IsPrivateHeader = false;
360-
return HeaderPath;
361-
}
362-
if (*I == "PrivateHeaders") {
363-
FrameworkHeaderPath HeaderPath;
364-
HeaderPath.HeadersParentDir = Path.substr(0, I - E);
365-
HeaderPath.HeaderSubpath = Path.substr(Prev - E);
366-
HeaderPath.IsPrivateHeader = true;
359+
HeaderPath.IsPrivateHeader = *I == "PrivateHeaders";
360+
if (++I == E)
361+
break;
362+
HeaderPath.FrameworkName = *I;
363+
if (!HeaderPath.FrameworkName.consume_back(".framework"))
364+
break;
365+
HeaderPath.FrameworkParentDir = Path.substr(0, I - E);
367366
return HeaderPath;
368367
}
369368
Prev = I;
@@ -379,26 +378,27 @@ class SymbolCollector::HeaderFileURICache {
379378
// <Foundation/NSObject_Private.h> which should be used instead of directly
380379
// importing the header.
381380
std::optional<std::string>
382-
getFrameworkUmbrellaSpelling(llvm::StringRef Framework,
383-
const HeaderSearch &HS,
381+
getFrameworkUmbrellaSpelling(const HeaderSearch &HS,
384382
FrameworkHeaderPath &HeaderPath) {
383+
StringRef Framework = HeaderPath.FrameworkName;
385384
auto Res = CacheFrameworkToUmbrellaHeaderSpelling.try_emplace(Framework);
386385
auto *CachedSpelling = &Res.first->second;
387386
if (!Res.second) {
388387
return HeaderPath.IsPrivateHeader ? CachedSpelling->PrivateHeader
389388
: CachedSpelling->PublicHeader;
390389
}
391-
SmallString<256> UmbrellaPath(HeaderPath.HeadersParentDir);
392-
llvm::sys::path::append(UmbrellaPath, "Headers", Framework + ".h");
390+
SmallString<256> UmbrellaPath(HeaderPath.FrameworkParentDir);
391+
llvm::sys::path::append(UmbrellaPath, Framework + ".framework", "Headers",
392+
Framework + ".h");
393393

394394
llvm::vfs::Status Status;
395395
auto StatErr = HS.getFileMgr().getNoncachedStatValue(UmbrellaPath, Status);
396396
if (!StatErr)
397397
CachedSpelling->PublicHeader = llvm::formatv("<{0}/{0}.h>", Framework);
398398

399-
UmbrellaPath = HeaderPath.HeadersParentDir;
400-
llvm::sys::path::append(UmbrellaPath, "PrivateHeaders",
401-
Framework + "_Private.h");
399+
UmbrellaPath = HeaderPath.FrameworkParentDir;
400+
llvm::sys::path::append(UmbrellaPath, Framework + ".framework",
401+
"PrivateHeaders", Framework + "_Private.h");
402402

403403
StatErr = HS.getFileMgr().getNoncachedStatValue(UmbrellaPath, Status);
404404
if (!StatErr)
@@ -414,8 +414,7 @@ class SymbolCollector::HeaderFileURICache {
414414
// give <Foundation/Foundation.h> if the umbrella header exists, otherwise
415415
// <Foundation/NSObject.h>.
416416
std::optional<llvm::StringRef>
417-
getFrameworkHeaderIncludeSpelling(FileEntryRef FE, llvm::StringRef Framework,
418-
HeaderSearch &HS) {
417+
getFrameworkHeaderIncludeSpelling(FileEntryRef FE, HeaderSearch &HS) {
419418
auto Res = CachePathToFrameworkSpelling.try_emplace(FE.getName());
420419
auto *CachedHeaderSpelling = &Res.first->second;
421420
if (!Res.second)
@@ -429,13 +428,15 @@ class SymbolCollector::HeaderFileURICache {
429428
return std::nullopt;
430429
}
431430
if (auto UmbrellaSpelling =
432-
getFrameworkUmbrellaSpelling(Framework, HS, *HeaderPath)) {
431+
getFrameworkUmbrellaSpelling(HS, *HeaderPath)) {
433432
*CachedHeaderSpelling = *UmbrellaSpelling;
434433
return llvm::StringRef(*CachedHeaderSpelling);
435434
}
436435

437436
*CachedHeaderSpelling =
438-
llvm::formatv("<{0}/{1}>", Framework, HeaderPath->HeaderSubpath).str();
437+
llvm::formatv("<{0}/{1}>", HeaderPath->FrameworkName,
438+
HeaderPath->HeaderSubpath)
439+
.str();
439440
return llvm::StringRef(*CachedHeaderSpelling);
440441
}
441442

@@ -454,11 +455,8 @@ class SymbolCollector::HeaderFileURICache {
454455
// Framework headers are spelled as <FrameworkName/Foo.h>, not
455456
// "path/FrameworkName.framework/Headers/Foo.h".
456457
auto &HS = PP->getHeaderSearchInfo();
457-
if (const auto *HFI = HS.getExistingFileInfo(*FE))
458-
if (!HFI->Framework.empty())
459-
if (auto Spelling =
460-
getFrameworkHeaderIncludeSpelling(*FE, HFI->Framework, HS))
461-
return *Spelling;
458+
if (auto Spelling = getFrameworkHeaderIncludeSpelling(*FE, HS))
459+
return *Spelling;
462460

463461
if (!tooling::isSelfContainedHeader(*FE, PP->getSourceManager(),
464462
PP->getHeaderSearchInfo())) {

‎clang/include/clang/Lex/HeaderSearch.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,6 @@ struct HeaderFileInfo {
122122
/// external storage.
123123
LazyIdentifierInfoPtr LazyControllingMacro;
124124

125-
/// If this header came from a framework include, this is the name
126-
/// of the framework.
127-
StringRef Framework;
128-
129125
HeaderFileInfo()
130126
: IsLocallyIncluded(false), isImport(false), isPragmaOnce(false),
131127
DirInfo(SrcMgr::C_User), External(false), isModuleHeader(false),
@@ -144,6 +140,8 @@ struct HeaderFileInfo {
144140
void mergeModuleMembership(ModuleMap::ModuleHeaderRole Role);
145141
};
146142

143+
static_assert(sizeof(HeaderFileInfo) <= 16);
144+
147145
/// An external source of header file information, which may supply
148146
/// information about header files already included.
149147
class ExternalHeaderFileInfoSource {

‎clang/include/clang/Serialization/ASTBitCodes.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ namespace serialization {
4444
/// Version 4 of AST files also requires that the version control branch and
4545
/// revision match exactly, since there is no backward compatibility of
4646
/// AST files at this time.
47-
const unsigned VERSION_MAJOR = 32;
47+
const unsigned VERSION_MAJOR = 33;
4848

4949
/// AST file minor version number supported by this version of
5050
/// Clang.

‎clang/lib/Lex/HeaderSearch.cpp

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -974,11 +974,9 @@ OptionalFileEntryRef HeaderSearch::LookupFile(
974974
const HeaderFileInfo *FromHFI = getExistingFileInfo(*Includer);
975975
assert(FromHFI && "includer without file info");
976976
unsigned DirInfo = FromHFI->DirInfo;
977-
StringRef Framework = FromHFI->Framework;
978977

979978
HeaderFileInfo &ToHFI = getFileInfo(*FE);
980979
ToHFI.DirInfo = DirInfo;
981-
ToHFI.Framework = Framework;
982980

983981
if (SearchPath) {
984982
StringRef SearchPathRef(IncluderAndDir.second.getName());
@@ -1120,16 +1118,6 @@ OptionalFileEntryRef HeaderSearch::LookupFile(
11201118
}
11211119
}
11221120

1123-
// Set the `Framework` info if this file is in a header map with framework
1124-
// style include spelling or found in a framework dir. The header map case
1125-
// is possible when building frameworks which use header maps.
1126-
if ((CurDir->isHeaderMap() && isAngled) || CurDir->isFramework()) {
1127-
size_t SlashPos = Filename.find('/');
1128-
if (SlashPos != StringRef::npos)
1129-
HFI.Framework =
1130-
getUniqueFrameworkName(StringRef(Filename.begin(), SlashPos));
1131-
}
1132-
11331121
if (checkMSVCHeaderSearch(Diags, MSFE, &File->getFileEntry(), IncludeLoc)) {
11341122
if (SuggestedModule)
11351123
*SuggestedModule = MSSuggestedModule;
@@ -1314,9 +1302,6 @@ static void mergeHeaderFileInfo(HeaderFileInfo &HFI,
13141302
HFI.DirInfo = OtherHFI.DirInfo;
13151303
HFI.External = (!HFI.IsValid || HFI.External);
13161304
HFI.IsValid = true;
1317-
1318-
if (HFI.Framework.empty())
1319-
HFI.Framework = OtherHFI.Framework;
13201305
}
13211306

13221307
HeaderFileInfo &HeaderSearch::getFileInfo(FileEntryRef FE) {

‎clang/lib/Serialization/ASTReader.cpp

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2122,13 +2122,6 @@ HeaderFileInfoTrait::ReadData(internal_key_ref key, const unsigned char *d,
21222122
HFI.DirInfo = (Flags >> 1) & 0x07;
21232123
HFI.LazyControllingMacro = Reader.getGlobalIdentifierID(
21242124
M, endian::readNext<IdentifierID, llvm::endianness::little>(d));
2125-
if (unsigned FrameworkOffset =
2126-
endian::readNext<uint32_t, llvm::endianness::little>(d)) {
2127-
// The framework offset is 1 greater than the actual offset,
2128-
// since 0 is used as an indicator for "no framework name".
2129-
StringRef FrameworkName(FrameworkStrings + FrameworkOffset - 1);
2130-
HFI.Framework = HS->getUniqueFrameworkName(FrameworkName);
2131-
}
21322125

21332126
assert((End - d) % 4 == 0 &&
21342127
"Wrong data length in HeaderFileInfo deserialization");
@@ -3902,13 +3895,10 @@ llvm::Error ASTReader::ReadASTBlock(ModuleFile &F,
39023895
F.HeaderFileInfoTableData = Blob.data();
39033896
F.LocalNumHeaderFileInfos = Record[1];
39043897
if (Record[0]) {
3905-
F.HeaderFileInfoTable
3906-
= HeaderFileInfoLookupTable::Create(
3907-
(const unsigned char *)F.HeaderFileInfoTableData + Record[0],
3908-
(const unsigned char *)F.HeaderFileInfoTableData,
3909-
HeaderFileInfoTrait(*this, F,
3910-
&PP.getHeaderSearchInfo(),
3911-
Blob.data() + Record[2]));
3898+
F.HeaderFileInfoTable = HeaderFileInfoLookupTable::Create(
3899+
(const unsigned char *)F.HeaderFileInfoTableData + Record[0],
3900+
(const unsigned char *)F.HeaderFileInfoTableData,
3901+
HeaderFileInfoTrait(*this, F));
39123902

39133903
PP.getHeaderSearchInfo().SetExternalSource(this);
39143904
if (!PP.getHeaderSearchInfo().getExternalLookup())

‎clang/lib/Serialization/ASTReaderInternals.h

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -243,8 +243,6 @@ using ASTSelectorLookupTable =
243243
class HeaderFileInfoTrait {
244244
ASTReader &Reader;
245245
ModuleFile &M;
246-
HeaderSearch *HS;
247-
const char *FrameworkStrings;
248246

249247
public:
250248
using external_key_type = FileEntryRef;
@@ -262,9 +260,8 @@ class HeaderFileInfoTrait {
262260
using hash_value_type = unsigned;
263261
using offset_type = unsigned;
264262

265-
HeaderFileInfoTrait(ASTReader &Reader, ModuleFile &M, HeaderSearch *HS,
266-
const char *FrameworkStrings)
267-
: Reader(Reader), M(M), HS(HS), FrameworkStrings(FrameworkStrings) {}
263+
HeaderFileInfoTrait(ASTReader &Reader, ModuleFile &M)
264+
: Reader(Reader), M(M) {}
268265

269266
static hash_value_type ComputeHash(internal_key_ref ikey);
270267
internal_key_type GetInternalKey(external_key_type ekey);

‎clang/lib/Serialization/ASTWriter.cpp

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1958,10 +1958,6 @@ namespace {
19581958
class HeaderFileInfoTrait {
19591959
ASTWriter &Writer;
19601960

1961-
// Keep track of the framework names we've used during serialization.
1962-
SmallString<128> FrameworkStringData;
1963-
llvm::StringMap<unsigned> FrameworkNameOffset;
1964-
19651961
public:
19661962
HeaderFileInfoTrait(ASTWriter &Writer) : Writer(Writer) {}
19671963

@@ -2005,7 +2001,7 @@ namespace {
20052001
std::pair<unsigned, unsigned>
20062002
EmitKeyDataLength(raw_ostream& Out, key_type_ref key, data_type_ref Data) {
20072003
unsigned KeyLen = key.Filename.size() + 1 + 8 + 8;
2008-
unsigned DataLen = 1 + sizeof(IdentifierID) + 4;
2004+
unsigned DataLen = 1 + sizeof(IdentifierID);
20092005
for (auto ModInfo : Data.KnownHeaders)
20102006
if (Writer.getLocalOrImportedSubmoduleID(ModInfo.getModule()))
20112007
DataLen += 4;
@@ -2045,22 +2041,6 @@ namespace {
20452041
LE.write<IdentifierID>(
20462042
Writer.getIdentifierRef(Data.HFI.LazyControllingMacro.getPtr()));
20472043

2048-
unsigned Offset = 0;
2049-
if (!Data.HFI.Framework.empty()) {
2050-
// If this header refers into a framework, save the framework name.
2051-
llvm::StringMap<unsigned>::iterator Pos
2052-
= FrameworkNameOffset.find(Data.HFI.Framework);
2053-
if (Pos == FrameworkNameOffset.end()) {
2054-
Offset = FrameworkStringData.size() + 1;
2055-
FrameworkStringData.append(Data.HFI.Framework);
2056-
FrameworkStringData.push_back(0);
2057-
2058-
FrameworkNameOffset[Data.HFI.Framework] = Offset;
2059-
} else
2060-
Offset = Pos->second;
2061-
}
2062-
LE.write<uint32_t>(Offset);
2063-
20642044
auto EmitModule = [&](Module *M, ModuleMap::ModuleHeaderRole Role) {
20652045
if (uint32_t ModID = Writer.getLocalOrImportedSubmoduleID(M)) {
20662046
uint32_t Value = (ModID << 3) | (unsigned)Role;
@@ -2076,9 +2056,6 @@ namespace {
20762056

20772057
assert(Out.tell() - Start == DataLen && "Wrong data length");
20782058
}
2079-
2080-
const char *strings_begin() const { return FrameworkStringData.begin(); }
2081-
const char *strings_end() const { return FrameworkStringData.end(); }
20822059
};
20832060

20842061
} // namespace
@@ -2213,7 +2190,6 @@ void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) {
22132190
// Write the header search table
22142191
RecordData::value_type Record[] = {HEADER_SEARCH_TABLE, BucketOffset,
22152192
NumHeaderSearchEntries, TableData.size()};
2216-
TableData.append(GeneratorTrait.strings_begin(),GeneratorTrait.strings_end());
22172193
Stream.EmitRecordWithBlob(TableAbbrev, Record, TableData);
22182194

22192195
// Free all of the strings we had to duplicate.

‎clang/unittests/Lex/HeaderSearchTest.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,6 @@ TEST_F(HeaderSearchTest, HeaderFrameworkLookup) {
250250
auto FI = Search.getExistingFileInfo(FE);
251251
EXPECT_TRUE(FI);
252252
EXPECT_TRUE(FI->IsValid);
253-
EXPECT_EQ(FI->Framework.str(), "Foo");
254253
EXPECT_EQ(Search.getIncludeNameForHeader(FE), "Foo/Foo.h");
255254
}
256255

@@ -320,7 +319,6 @@ TEST_F(HeaderSearchTest, HeaderMapFrameworkLookup) {
320319
auto FI = Search.getExistingFileInfo(FE);
321320
EXPECT_TRUE(FI);
322321
EXPECT_TRUE(FI->IsValid);
323-
EXPECT_EQ(FI->Framework.str(), "Foo");
324322
EXPECT_EQ(Search.getIncludeNameForHeader(FE), "Foo/Foo.h");
325323
}
326324

0 commit comments

Comments
 (0)