Skip to content

Commit bcc4ba9

Browse files
authored
Request Evaluator: save malloc traffic by canonicalizing AnyRequests (#20999)
Use the Dependencies map to standardize on a single allocation for any particular Request. We could probably go all the way to unique ownership here, but I didn't want to rock the boat. Results in minor speedups compiling the stdlib. (I was experimenting with using a new, auto-cached Request for a hot lookup path, which may or may not turn out to be worth it, but even without that we can take this.)
1 parent facaad1 commit bcc4ba9

File tree

1 file changed

+14
-4
lines changed

1 file changed

+14
-4
lines changed

include/swift/AST/Evaluator.h

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ class Evaluator {
259259
llvm::Expected<typename Request::OutputType>
260260
operator()(const Request &request) {
261261
// Check for a cycle.
262-
if (checkDependency(AnyRequest(request))) {
262+
if (checkDependency(getCanonicalRequest(request))) {
263263
return llvm::Error(
264264
llvm::make_unique<CyclicalRequestError<Request>>(request, *this));
265265
}
@@ -293,6 +293,17 @@ class Evaluator {
293293
void clearCache() { cache.clear(); }
294294

295295
private:
296+
template <typename Request>
297+
const AnyRequest &getCanonicalRequest(const Request &request) {
298+
// FIXME: DenseMap ought to let us do this with one hash lookup.
299+
auto iter = dependencies.find_as(request);
300+
if (iter != dependencies.end())
301+
return iter->first;
302+
auto insertResult = dependencies.insert({AnyRequest(request), {}});
303+
assert(insertResult.second && "just checked if the key was already there");
304+
return insertResult.first->first;
305+
}
306+
296307
/// Diagnose a cycle detected in the evaluation of the given
297308
/// request.
298309
void diagnoseCycle(const AnyRequest &request);
@@ -335,7 +346,7 @@ class Evaluator {
335346
getResultUncached(const Request &request) {
336347
// Clear out the dependencies on this request; we're going to recompute
337348
// them now anyway.
338-
dependencies[AnyRequest(request)].clear();
349+
dependencies.find_as(request)->second.clear();
339350

340351
PrettyStackTraceRequest<Request> prettyStackTrace(request);
341352

@@ -377,7 +388,6 @@ class Evaluator {
377388
typename std::enable_if<!Request::hasExternalCache>::type * = nullptr>
378389
llvm::Expected<typename Request::OutputType>
379390
getResultCached(const Request &request) {
380-
AnyRequest anyRequest{request};
381391
// If we already have an entry for this request in the cache, return it.
382392
auto known = cache.find_as(request);
383393
if (known != cache.end()) {
@@ -390,7 +400,7 @@ class Evaluator {
390400
return result;
391401

392402
// Cache the result.
393-
cache.insert({AnyRequest(request), *result});
403+
cache.insert({getCanonicalRequest(request), *result});
394404
return result;
395405
}
396406

0 commit comments

Comments
 (0)