Skip to content

Commit 65e57bb

Browse files
teresajohnsonteresajohnson
committed
[FunctionImport] Reduce string duplication (NFC)
The import/export maps, and the ModuleToDefinedGVSummaries map, are all indexed by module paths, which are StringRef obtained from the module summary index, which already has a data structure than owns these strings (the ModulePathStringTable). Because these other maps are also StringMap, which makes a copy of the string key, we were keeping multiple extra copies of the module paths, leading to memory overhead. Change these to DenseMap keyed by StringRef, and document that the strings are owned by the index. The only exception is the llvm-link tool which synthesizes an import list from command line options, and I have added a string cache to maintain ownership there. I measured around 5% memory reduction in the thin link of a large binary. Differential Revision: https://reviews.llvm.org/D156580
1 parent 68f3610 commit 65e57bb

File tree

7 files changed

+99
-81
lines changed

7 files changed

+99
-81
lines changed

‎clang/lib/CodeGen/BackendUtil.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -1169,7 +1169,7 @@ static void runThinLTOBackend(
11691169
const clang::TargetOptions &TOpts, const LangOptions &LOpts,
11701170
std::unique_ptr<raw_pwrite_stream> OS, std::string SampleProfile,
11711171
std::string ProfileRemapping, BackendAction Action) {
1172-
StringMap<DenseMap<GlobalValue::GUID, GlobalValueSummary *>>
1172+
DenseMap<StringRef, DenseMap<GlobalValue::GUID, GlobalValueSummary *>>
11731173
ModuleToDefinedGVSummaries;
11741174
CombinedIndex->collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
11751175

‎llvm/include/llvm/LTO/LTO.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ class InputFile {
196196
/// create a ThinBackend using one of the create*ThinBackend() functions below.
197197
using ThinBackend = std::function<std::unique_ptr<ThinBackendProc>(
198198
const Config &C, ModuleSummaryIndex &CombinedIndex,
199-
StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries,
199+
DenseMap<StringRef, GVSummaryMapTy> &ModuleToDefinedGVSummaries,
200200
AddStreamFn AddStream, FileCache Cache)>;
201201

202202
/// This ThinBackend runs the individual backend jobs in-process.

‎llvm/include/llvm/Transforms/IPO/FunctionImport.h

+14-6
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,11 @@ class FunctionImporter {
9494

9595
/// The map contains an entry for every module to import from, the key being
9696
/// the module identifier to pass to the ModuleLoader. The value is the set of
97-
/// functions to import.
98-
using ImportMapTy = StringMap<FunctionsToImportTy>;
97+
/// functions to import. The module identifier strings must be owned
98+
/// elsewhere, typically by the in-memory ModuleSummaryIndex the importing
99+
/// decisions are made from (the module path for each summary is owned by the
100+
/// index's module path string table).
101+
using ImportMapTy = DenseMap<StringRef, FunctionsToImportTy>;
99102

100103
/// The set contains an entry for every global value the module exports.
101104
using ExportSetTy = DenseSet<ValueInfo>;
@@ -147,13 +150,18 @@ class FunctionImportPass : public PassInfoMixin<FunctionImportPass> {
147150
/// \p ExportLists contains for each Module the set of globals (GUID) that will
148151
/// be imported by another module, or referenced by such a function. I.e. this
149152
/// is the set of globals that need to be promoted/renamed appropriately.
153+
///
154+
/// The module identifier strings that are the keys of the above two maps
155+
/// are owned by the in-memory ModuleSummaryIndex the importing decisions
156+
/// are made from (the module path for each summary is owned by the index's
157+
/// module path string table).
150158
void ComputeCrossModuleImport(
151159
const ModuleSummaryIndex &Index,
152-
const StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries,
160+
const DenseMap<StringRef, GVSummaryMapTy> &ModuleToDefinedGVSummaries,
153161
function_ref<bool(GlobalValue::GUID, const GlobalValueSummary *)>
154162
isPrevailing,
155-
StringMap<FunctionImporter::ImportMapTy> &ImportLists,
156-
StringMap<FunctionImporter::ExportSetTy> &ExportLists);
163+
DenseMap<StringRef, FunctionImporter::ImportMapTy> &ImportLists,
164+
DenseMap<StringRef, FunctionImporter::ExportSetTy> &ExportLists);
157165

158166
/// Compute all the imports for the given module using the Index.
159167
///
@@ -225,7 +233,7 @@ bool convertToDeclaration(GlobalValue &GV);
225233
/// stable order for bitcode emission.
226234
void gatherImportedSummariesForModule(
227235
StringRef ModulePath,
228-
const StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries,
236+
const DenseMap<StringRef, GVSummaryMapTy> &ModuleToDefinedGVSummaries,
229237
const FunctionImporter::ImportMapTy &ImportList,
230238
std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex);
231239

‎llvm/lib/LTO/LTO.cpp

+31-26
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ void llvm::computeLTOCacheKey(
178178
ImportMapIteratorTy ModIt;
179179
const ModuleSummaryIndex::ModuleInfo *ModInfo;
180180

181-
StringRef getIdentifier() const { return ModIt->getKey(); }
181+
StringRef getIdentifier() const { return ModIt->getFirst(); }
182182
const FunctionImporter::FunctionsToImportTy &getFunctions() const {
183183
return ModIt->second;
184184
}
@@ -191,7 +191,7 @@ void llvm::computeLTOCacheKey(
191191

192192
for (ImportMapIteratorTy It = ImportList.begin(); It != ImportList.end();
193193
++It) {
194-
ImportModulesVector.push_back({It, Index.getModule(It->getKey())});
194+
ImportModulesVector.push_back({It, Index.getModule(It->getFirst())});
195195
}
196196
// Order using module hash, to be both independent of module name and
197197
// module order.
@@ -1362,14 +1362,15 @@ class lto::ThinBackendProc {
13621362
protected:
13631363
const Config &Conf;
13641364
ModuleSummaryIndex &CombinedIndex;
1365-
const StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries;
1365+
const DenseMap<StringRef, GVSummaryMapTy> &ModuleToDefinedGVSummaries;
13661366
lto::IndexWriteCallback OnWrite;
13671367
bool ShouldEmitImportsFiles;
13681368

13691369
public:
1370-
ThinBackendProc(const Config &Conf, ModuleSummaryIndex &CombinedIndex,
1371-
const StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries,
1372-
lto::IndexWriteCallback OnWrite, bool ShouldEmitImportsFiles)
1370+
ThinBackendProc(
1371+
const Config &Conf, ModuleSummaryIndex &CombinedIndex,
1372+
const DenseMap<StringRef, GVSummaryMapTy> &ModuleToDefinedGVSummaries,
1373+
lto::IndexWriteCallback OnWrite, bool ShouldEmitImportsFiles)
13731374
: Conf(Conf), CombinedIndex(CombinedIndex),
13741375
ModuleToDefinedGVSummaries(ModuleToDefinedGVSummaries),
13751376
OnWrite(OnWrite), ShouldEmitImportsFiles(ShouldEmitImportsFiles) {}
@@ -1426,7 +1427,7 @@ class InProcessThinBackend : public ThinBackendProc {
14261427
InProcessThinBackend(
14271428
const Config &Conf, ModuleSummaryIndex &CombinedIndex,
14281429
ThreadPoolStrategy ThinLTOParallelism,
1429-
const StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries,
1430+
const DenseMap<StringRef, GVSummaryMapTy> &ModuleToDefinedGVSummaries,
14301431
AddStreamFn AddStream, FileCache Cache, lto::IndexWriteCallback OnWrite,
14311432
bool ShouldEmitIndexFiles, bool ShouldEmitImportsFiles)
14321433
: ThinBackendProc(Conf, CombinedIndex, ModuleToDefinedGVSummaries,
@@ -1548,13 +1549,15 @@ ThinBackend lto::createInProcessThinBackend(ThreadPoolStrategy Parallelism,
15481549
lto::IndexWriteCallback OnWrite,
15491550
bool ShouldEmitIndexFiles,
15501551
bool ShouldEmitImportsFiles) {
1551-
return [=](const Config &Conf, ModuleSummaryIndex &CombinedIndex,
1552-
const StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries,
1553-
AddStreamFn AddStream, FileCache Cache) {
1554-
return std::make_unique<InProcessThinBackend>(
1555-
Conf, CombinedIndex, Parallelism, ModuleToDefinedGVSummaries, AddStream,
1556-
Cache, OnWrite, ShouldEmitIndexFiles, ShouldEmitImportsFiles);
1557-
};
1552+
return
1553+
[=](const Config &Conf, ModuleSummaryIndex &CombinedIndex,
1554+
const DenseMap<StringRef, GVSummaryMapTy> &ModuleToDefinedGVSummaries,
1555+
AddStreamFn AddStream, FileCache Cache) {
1556+
return std::make_unique<InProcessThinBackend>(
1557+
Conf, CombinedIndex, Parallelism, ModuleToDefinedGVSummaries,
1558+
AddStream, Cache, OnWrite, ShouldEmitIndexFiles,
1559+
ShouldEmitImportsFiles);
1560+
};
15581561
}
15591562

15601563
// Given the original \p Path to an output file, replace any path
@@ -1584,7 +1587,7 @@ class WriteIndexesThinBackend : public ThinBackendProc {
15841587
public:
15851588
WriteIndexesThinBackend(
15861589
const Config &Conf, ModuleSummaryIndex &CombinedIndex,
1587-
const StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries,
1590+
const DenseMap<StringRef, GVSummaryMapTy> &ModuleToDefinedGVSummaries,
15881591
std::string OldPrefix, std::string NewPrefix,
15891592
std::string NativeObjectPrefix, bool ShouldEmitImportsFiles,
15901593
raw_fd_ostream *LinkedObjectsFile, lto::IndexWriteCallback OnWrite)
@@ -1632,13 +1635,15 @@ ThinBackend lto::createWriteIndexesThinBackend(
16321635
std::string OldPrefix, std::string NewPrefix,
16331636
std::string NativeObjectPrefix, bool ShouldEmitImportsFiles,
16341637
raw_fd_ostream *LinkedObjectsFile, IndexWriteCallback OnWrite) {
1635-
return [=](const Config &Conf, ModuleSummaryIndex &CombinedIndex,
1636-
const StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries,
1637-
AddStreamFn AddStream, FileCache Cache) {
1638-
return std::make_unique<WriteIndexesThinBackend>(
1639-
Conf, CombinedIndex, ModuleToDefinedGVSummaries, OldPrefix, NewPrefix,
1640-
NativeObjectPrefix, ShouldEmitImportsFiles, LinkedObjectsFile, OnWrite);
1641-
};
1638+
return
1639+
[=](const Config &Conf, ModuleSummaryIndex &CombinedIndex,
1640+
const DenseMap<StringRef, GVSummaryMapTy> &ModuleToDefinedGVSummaries,
1641+
AddStreamFn AddStream, FileCache Cache) {
1642+
return std::make_unique<WriteIndexesThinBackend>(
1643+
Conf, CombinedIndex, ModuleToDefinedGVSummaries, OldPrefix,
1644+
NewPrefix, NativeObjectPrefix, ShouldEmitImportsFiles,
1645+
LinkedObjectsFile, OnWrite);
1646+
};
16421647
}
16431648

16441649
Error LTO::runThinLTO(AddStreamFn AddStream, FileCache Cache,
@@ -1664,8 +1669,8 @@ Error LTO::runThinLTO(AddStreamFn AddStream, FileCache Cache,
16641669

16651670
// Collect for each module the list of function it defines (GUID ->
16661671
// Summary).
1667-
StringMap<GVSummaryMapTy>
1668-
ModuleToDefinedGVSummaries(ThinLTO.ModuleMap.size());
1672+
DenseMap<StringRef, GVSummaryMapTy> ModuleToDefinedGVSummaries(
1673+
ThinLTO.ModuleMap.size());
16691674
ThinLTO.CombinedIndex.collectDefinedGVSummariesPerModule(
16701675
ModuleToDefinedGVSummaries);
16711676
// Create entries for any modules that didn't have any GV summaries
@@ -1682,9 +1687,9 @@ Error LTO::runThinLTO(AddStreamFn AddStream, FileCache Cache,
16821687
// Synthesize entry counts for functions in the CombinedIndex.
16831688
computeSyntheticCounts(ThinLTO.CombinedIndex);
16841689

1685-
StringMap<FunctionImporter::ImportMapTy> ImportLists(
1690+
DenseMap<StringRef, FunctionImporter::ImportMapTy> ImportLists(
16861691
ThinLTO.ModuleMap.size());
1687-
StringMap<FunctionImporter::ExportSetTy> ExportLists(
1692+
DenseMap<StringRef, FunctionImporter::ExportSetTy> ExportLists(
16881693
ThinLTO.ModuleMap.size());
16891694
StringMap<std::map<GlobalValue::GUID, GlobalValue::LinkageTypes>> ResolvedODR;
16901695

‎llvm/lib/LTO/ThinLTOCodeGenerator.cpp

+22-21
Original file line numberDiff line numberDiff line change
@@ -634,11 +634,12 @@ std::unique_ptr<ModuleSummaryIndex> ThinLTOCodeGenerator::linkCombinedIndex() {
634634

635635
namespace {
636636
struct IsExported {
637-
const StringMap<FunctionImporter::ExportSetTy> &ExportLists;
637+
const DenseMap<StringRef, FunctionImporter::ExportSetTy> &ExportLists;
638638
const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols;
639639

640-
IsExported(const StringMap<FunctionImporter::ExportSetTy> &ExportLists,
641-
const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols)
640+
IsExported(
641+
const DenseMap<StringRef, FunctionImporter::ExportSetTy> &ExportLists,
642+
const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols)
642643
: ExportLists(ExportLists), GUIDPreservedSymbols(GUIDPreservedSymbols) {}
643644

644645
bool operator()(StringRef ModuleIdentifier, ValueInfo VI) const {
@@ -687,7 +688,7 @@ void ThinLTOCodeGenerator::promote(Module &TheModule, ModuleSummaryIndex &Index,
687688
auto ModuleIdentifier = TheModule.getModuleIdentifier();
688689

689690
// Collect for each module the list of function it defines (GUID -> Summary).
690-
StringMap<GVSummaryMapTy> ModuleToDefinedGVSummaries;
691+
DenseMap<StringRef, GVSummaryMapTy> ModuleToDefinedGVSummaries;
691692
Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
692693

693694
// Convert the preserved symbols set from string to GUID
@@ -705,8 +706,8 @@ void ThinLTOCodeGenerator::promote(Module &TheModule, ModuleSummaryIndex &Index,
705706
computePrevailingCopies(Index, PrevailingCopy);
706707

707708
// Generate import/export list
708-
StringMap<FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
709-
StringMap<FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
709+
DenseMap<StringRef, FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
710+
DenseMap<StringRef, FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
710711
ComputeCrossModuleImport(Index, ModuleToDefinedGVSummaries,
711712
IsPrevailing(PrevailingCopy), ImportLists,
712713
ExportLists);
@@ -740,7 +741,7 @@ void ThinLTOCodeGenerator::crossModuleImport(Module &TheModule,
740741
auto ModuleCount = Index.modulePaths().size();
741742

742743
// Collect for each module the list of function it defines (GUID -> Summary).
743-
StringMap<GVSummaryMapTy> ModuleToDefinedGVSummaries(ModuleCount);
744+
DenseMap<StringRef, GVSummaryMapTy> ModuleToDefinedGVSummaries(ModuleCount);
744745
Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
745746

746747
// Convert the preserved symbols set from string to GUID
@@ -757,8 +758,8 @@ void ThinLTOCodeGenerator::crossModuleImport(Module &TheModule,
757758
computePrevailingCopies(Index, PrevailingCopy);
758759

759760
// Generate import/export list
760-
StringMap<FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
761-
StringMap<FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
761+
DenseMap<StringRef, FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
762+
DenseMap<StringRef, FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
762763
ComputeCrossModuleImport(Index, ModuleToDefinedGVSummaries,
763764
IsPrevailing(PrevailingCopy), ImportLists,
764765
ExportLists);
@@ -780,7 +781,7 @@ void ThinLTOCodeGenerator::gatherImportedSummariesForModule(
780781
auto ModuleIdentifier = TheModule.getModuleIdentifier();
781782

782783
// Collect for each module the list of function it defines (GUID -> Summary).
783-
StringMap<GVSummaryMapTy> ModuleToDefinedGVSummaries(ModuleCount);
784+
DenseMap<StringRef, GVSummaryMapTy> ModuleToDefinedGVSummaries(ModuleCount);
784785
Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
785786

786787
// Convert the preserved symbols set from string to GUID
@@ -797,8 +798,8 @@ void ThinLTOCodeGenerator::gatherImportedSummariesForModule(
797798
computePrevailingCopies(Index, PrevailingCopy);
798799

799800
// Generate import/export list
800-
StringMap<FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
801-
StringMap<FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
801+
DenseMap<StringRef, FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
802+
DenseMap<StringRef, FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
802803
ComputeCrossModuleImport(Index, ModuleToDefinedGVSummaries,
803804
IsPrevailing(PrevailingCopy), ImportLists,
804805
ExportLists);
@@ -818,7 +819,7 @@ void ThinLTOCodeGenerator::emitImports(Module &TheModule, StringRef OutputName,
818819
auto ModuleIdentifier = TheModule.getModuleIdentifier();
819820

820821
// Collect for each module the list of function it defines (GUID -> Summary).
821-
StringMap<GVSummaryMapTy> ModuleToDefinedGVSummaries(ModuleCount);
822+
DenseMap<StringRef, GVSummaryMapTy> ModuleToDefinedGVSummaries(ModuleCount);
822823
Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
823824

824825
// Convert the preserved symbols set from string to GUID
@@ -835,8 +836,8 @@ void ThinLTOCodeGenerator::emitImports(Module &TheModule, StringRef OutputName,
835836
computePrevailingCopies(Index, PrevailingCopy);
836837

837838
// Generate import/export list
838-
StringMap<FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
839-
StringMap<FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
839+
DenseMap<StringRef, FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
840+
DenseMap<StringRef, FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
840841
ComputeCrossModuleImport(Index, ModuleToDefinedGVSummaries,
841842
IsPrevailing(PrevailingCopy), ImportLists,
842843
ExportLists);
@@ -871,7 +872,7 @@ void ThinLTOCodeGenerator::internalize(Module &TheModule,
871872
addUsedSymbolToPreservedGUID(File, GUIDPreservedSymbols);
872873

873874
// Collect for each module the list of function it defines (GUID -> Summary).
874-
StringMap<GVSummaryMapTy> ModuleToDefinedGVSummaries(ModuleCount);
875+
DenseMap<StringRef, GVSummaryMapTy> ModuleToDefinedGVSummaries(ModuleCount);
875876
Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
876877

877878
// Compute "dead" symbols, we don't want to import/export these!
@@ -882,8 +883,8 @@ void ThinLTOCodeGenerator::internalize(Module &TheModule,
882883
computePrevailingCopies(Index, PrevailingCopy);
883884

884885
// Generate import/export list
885-
StringMap<FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
886-
StringMap<FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
886+
DenseMap<StringRef, FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
887+
DenseMap<StringRef, FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
887888
ComputeCrossModuleImport(Index, ModuleToDefinedGVSummaries,
888889
IsPrevailing(PrevailingCopy), ImportLists,
889890
ExportLists);
@@ -1033,7 +1034,7 @@ void ThinLTOCodeGenerator::run() {
10331034
auto ModuleCount = Modules.size();
10341035

10351036
// Collect for each module the list of function it defines (GUID -> Summary).
1036-
StringMap<GVSummaryMapTy> ModuleToDefinedGVSummaries(ModuleCount);
1037+
DenseMap<StringRef, GVSummaryMapTy> ModuleToDefinedGVSummaries(ModuleCount);
10371038
Index->collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
10381039

10391040
// Convert the preserved symbols set from string to GUID, this is needed for
@@ -1079,8 +1080,8 @@ void ThinLTOCodeGenerator::run() {
10791080

10801081
// Collect the import/export lists for all modules from the call-graph in the
10811082
// combined index.
1082-
StringMap<FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
1083-
StringMap<FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
1083+
DenseMap<StringRef, FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
1084+
DenseMap<StringRef, FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
10841085
ComputeCrossModuleImport(*Index, ModuleToDefinedGVSummaries,
10851086
IsPrevailing(PrevailingCopy), ImportLists,
10861087
ExportLists);

0 commit comments

Comments
 (0)