Skip to content

Commit 5adb69e

Browse files
committed
[Serialization] Don't read all declaration id eagerly when merge the tables
See the post commit message in llvm#92083 for rationale. Previously, when we merge the lookup tables, we will read the tables completely to get the data. But the above page shows that it might be problematic in specific cases where there a lot of DeclIDs to be read. In this patch, we mitigated the problem by not reading the contents of the merged tables. But we may need to pay for the additional memory usages.
1 parent 78ee473 commit 5adb69e

File tree

1 file changed

+54
-10
lines changed

1 file changed

+54
-10
lines changed

clang/lib/Serialization/MultiOnDiskHashTable.h

+54-10
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,51 @@ template<typename Info> class MultiOnDiskHashTable {
7373
struct MergedTable {
7474
std::vector<file_type> Files;
7575
llvm::DenseMap<internal_key_type, data_type> Data;
76+
77+
struct UnreadDataInfo {
78+
Info *InfoObj;
79+
const unsigned char *start;
80+
unsigned DataLen;
81+
};
82+
83+
llvm::DenseMap<internal_key_type, llvm::SmallVector<UnreadDataInfo, 16>>
84+
UnReadData;
85+
86+
std::vector<OnDiskTable *> MergedOnDiskTables;
87+
88+
void clear() {
89+
for (OnDiskTable *ODT : MergedOnDiskTables)
90+
delete ODT;
91+
92+
MergedOnDiskTables.clear();
93+
}
94+
95+
void readAll() {
96+
for (const auto &Iter : UnReadData) {
97+
internal_key_type Key = Iter.first;
98+
data_type_builder ValueBuilder(Data[Key]);
99+
100+
for (const UnreadDataInfo &I : Iter.second)
101+
I.InfoObj->ReadDataInto(Key, I.start, I.DataLen, ValueBuilder);
102+
}
103+
104+
UnReadData.clear();
105+
clear();
106+
}
107+
108+
data_type find(internal_key_type Key) {
109+
auto UnreadIter = UnReadData.find(Key);
110+
if (UnreadIter != UnReadData.end()) {
111+
data_type_builder ValueBuilder(Data[Key]);
112+
for (auto &I : UnreadIter->second)
113+
I.InfoObj->ReadDataInto(Key, I.start, I.DataLen, ValueBuilder);
114+
UnReadData.erase(UnreadIter);
115+
}
116+
117+
return Data[Key];
118+
}
119+
120+
~MergedTable() { clear(); }
76121
};
77122

78123
using Table = llvm::PointerUnion<OnDiskTable *, MergedTable *>;
@@ -159,13 +204,14 @@ template<typename Info> class MultiOnDiskHashTable {
159204
// FIXME: Don't rely on the OnDiskHashTable format here.
160205
auto L = InfoObj.ReadKeyDataLength(LocalPtr);
161206
const internal_key_type &Key = InfoObj.ReadKey(LocalPtr, L.first);
162-
data_type_builder ValueBuilder(Merged->Data[Key]);
163-
InfoObj.ReadDataInto(Key, LocalPtr + L.first, L.second,
164-
ValueBuilder);
207+
// Make sure Merged->Data contains every key.
208+
(void)Merged->Data[Key];
209+
Merged->UnReadData[Key].push_back(
210+
{&InfoObj, LocalPtr + L.first, L.second});
165211
}
166212

167213
Merged->Files.push_back(ODT->File);
168-
delete ODT;
214+
Merged->MergedOnDiskTables.push_back(ODT);
169215
}
170216

171217
Tables.clear();
@@ -239,11 +285,8 @@ template<typename Info> class MultiOnDiskHashTable {
239285
internal_key_type Key = Info::GetInternalKey(EKey);
240286
auto KeyHash = Info::ComputeHash(Key);
241287

242-
if (MergedTable *M = getMergedTable()) {
243-
auto It = M->Data.find(Key);
244-
if (It != M->Data.end())
245-
Result = It->second;
246-
}
288+
if (MergedTable *M = getMergedTable())
289+
Result = M->find(Key);
247290

248291
data_type_builder ResultBuilder(Result);
249292

@@ -268,6 +311,7 @@ template<typename Info> class MultiOnDiskHashTable {
268311
removeOverriddenTables();
269312

270313
if (MergedTable *M = getMergedTable()) {
314+
M->readAll();
271315
for (auto &KV : M->Data)
272316
Info::MergeDataInto(KV.second, ResultBuilder);
273317
}
@@ -327,7 +371,7 @@ class MultiOnDiskHashTableGenerator {
327371
// Add all merged entries from Base to the generator.
328372
for (auto &KV : Merged->Data) {
329373
if (!Gen.contains(KV.first, Info))
330-
Gen.insert(KV.first, Info.ImportData(KV.second), Info);
374+
Gen.insert(KV.first, Info.ImportData(Merged->find(KV.first)), Info);
331375
}
332376
} else {
333377
Writer.write<uint32_t>(0);

0 commit comments

Comments
 (0)