Skip to content

Commit c56f3c6

Browse files
committed
Request evaluator: Use DenseMap::find_as to look up Requests
...when we don't need to store them, to avoid copying the Request into an AnyRequest heap box.
1 parent 0387798 commit c56f3c6

File tree

3 files changed

+39
-23
lines changed

3 files changed

+39
-23
lines changed

include/swift/AST/AnyRequest.h

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ class DiagnosticEngine;
5050
/// void noteCycleStep(DiagnosticEngine &diags) const;
5151
///
5252
class AnyRequest {
53+
friend llvm::DenseMapInfo<swift::AnyRequest>;
54+
55+
static hash_code hashForHolder(uint64_t typeID, hash_code requestHash) {
56+
return hash_combine(hash_value(typeID), requestHash);
57+
}
58+
5359
/// Abstract base class used to hold the specific request kind.
5460
class HolderBase : public llvm::RefCountedBase<HolderBase> {
5561
public:
@@ -62,7 +68,7 @@ class AnyRequest {
6268
protected:
6369
/// Initialize base with type ID and hash code.
6470
HolderBase(uint64_t typeID, hash_code hash)
65-
: typeID(typeID), hash(hash_combine(hash_value(typeID), hash)) { }
71+
: typeID(typeID), hash(AnyRequest::hashForHolder(typeID, hash)) { }
6672

6773
public:
6874
virtual ~HolderBase();
@@ -160,7 +166,7 @@ class AnyRequest {
160166

161167
/// Construct a new instance with the given value.
162168
template<typename T>
163-
AnyRequest(T&& value) : storageKind(StorageKind::Normal) {
169+
explicit AnyRequest(T&& value) : storageKind(StorageKind::Normal) {
164170
using ValueType =
165171
typename std::remove_cv<typename std::remove_reference<T>::type>::type;
166172
stored = llvm::IntrusiveRefCntPtr<HolderBase>(
@@ -171,7 +177,7 @@ class AnyRequest {
171177
template<typename Request>
172178
const Request &castTo() const {
173179
assert(stored->typeID == TypeID<Request>::value && "wrong type in cast");
174-
return static_cast<const Holder<Request> *>(stored.get())->value;
180+
return static_cast<const Holder<Request> *>(stored.get())->request;
175181
}
176182

177183
/// Try casting to a specific (known) type, returning \c nullptr on
@@ -181,7 +187,7 @@ class AnyRequest {
181187
if (stored->typeID != TypeID<Request>::value)
182188
return nullptr;
183189

184-
return &static_cast<const Holder<Request> *>(stored.get())->value;
190+
return &static_cast<const Holder<Request> *>(stored.get())->request;
185191
}
186192

187193
/// Diagnose a cycle detected for this request.
@@ -250,10 +256,25 @@ namespace llvm {
250256
static unsigned getHashValue(const swift::AnyRequest &request) {
251257
return hash_value(request);
252258
}
259+
template <typename Request>
260+
static unsigned getHashValue(const Request &request) {
261+
return swift::AnyRequest::hashForHolder(swift::TypeID<Request>::value,
262+
hash_value(request));
263+
}
253264
static bool isEqual(const swift::AnyRequest &lhs,
254265
const swift::AnyRequest &rhs) {
255266
return lhs == rhs;
256267
}
268+
template <typename Request>
269+
static bool isEqual(const Request &lhs,
270+
const swift::AnyRequest &rhs) {
271+
if (rhs == getEmptyKey() || rhs == getTombstoneKey())
272+
return false;
273+
const Request *rhsRequest = rhs.getAs<Request>();
274+
if (!rhsRequest)
275+
return false;
276+
return lhs == *rhsRequest;
277+
}
257278
};
258279

259280
} // end namespace llvm

include/swift/AST/Evaluator.h

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -213,13 +213,13 @@ class Evaluator {
213213
template<typename Request>
214214
typename Request::OutputType operator()(const Request &request) {
215215
// Check for a cycle.
216-
if (checkDependency(request))
216+
if (checkDependency(AnyRequest(request)))
217217
return request.breakCycle();
218218

219219
// Make sure we remove this from the set of active requests once we're
220220
// done.
221221
SWIFT_DEFER {
222-
assert(activeRequests.back() == request);
222+
assert(activeRequests.back().castTo<Request>() == request);
223223
activeRequests.pop_back();
224224
};
225225

@@ -283,7 +283,7 @@ class Evaluator {
283283
typename Request::OutputType getResultUncached(const Request &request) {
284284
// Clear out the dependencies on this request; we're going to recompute
285285
// them now anyway.
286-
dependencies[request].clear();
286+
dependencies[AnyRequest(request)].clear();
287287

288288
PrettyStackTraceRequest<Request> prettyStackTrace(request);
289289

@@ -319,19 +319,17 @@ class Evaluator {
319319
typename Request,
320320
typename std::enable_if<!Request::hasExternalCache>::type * = nullptr>
321321
typename Request::OutputType getResultCached(const Request &request) {
322-
AnyRequest anyRequest{request};
323-
324322
// If we already have an entry for this request in the cache, return it.
325-
auto known = cache.find(anyRequest);
323+
auto known = cache.find_as(request);
326324
if (known != cache.end()) {
327-
return known->second.castTo<typename Request::OutputType>();
325+
return known->second.template castTo<typename Request::OutputType>();
328326
}
329327

330328
// Compute the result.
331329
auto result = getResultUncached(request);
332330

333331
// Cache the result.
334-
cache.insert({anyRequest, result});
332+
cache.insert({AnyRequest(request), result});
335333
return result;
336334
}
337335

@@ -349,8 +347,14 @@ class Evaluator {
349347
bool lastChild) const;
350348

351349
/// Print the dependencies of the given request as a tree.
352-
void printDependencies(const AnyRequest &request,
353-
llvm::raw_ostream &out) const;
350+
template <typename Request>
351+
void printDependencies(const Request &request, llvm::raw_ostream &out) const {
352+
std::string prefixStr;
353+
llvm::DenseSet<AnyRequest> visitedAnywhere;
354+
llvm::SmallVector<AnyRequest, 4> visitedAlongPath;
355+
printDependencies(AnyRequest(request), out, visitedAnywhere,
356+
visitedAlongPath, { }, prefixStr, /*lastChild=*/true);
357+
}
354358

355359
/// Dump the dependencies of the given request to the debugging stream
356360
/// as a tree.

lib/AST/Evaluator.cpp

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -191,15 +191,6 @@ void Evaluator::printDependencies(
191191
}
192192
}
193193

194-
void Evaluator::printDependencies(const AnyRequest &request,
195-
llvm::raw_ostream &out) const {
196-
std::string prefixStr;
197-
llvm::DenseSet<AnyRequest> visitedAnywhere;
198-
llvm::SmallVector<AnyRequest, 4> visitedAlongPath;
199-
printDependencies(request, out, visitedAnywhere, visitedAlongPath, { },
200-
prefixStr, /*lastChild=*/true);
201-
}
202-
203194
void Evaluator::dumpDependencies(const AnyRequest &request) const {
204195
printDependencies(request, llvm::dbgs());
205196
}

0 commit comments

Comments
 (0)