Skip to content

Commit 9bcdade

Browse files
committed
AST: Look for cached request result before checking for a cycle
This improves performance in the common case where the result has already been cached, because the cycle check constructs an AnyRequest existential, which is expensive.
1 parent 01ebc93 commit 9bcdade

File tree

1 file changed

+31
-40
lines changed

1 file changed

+31
-40
lines changed

include/swift/AST/Evaluator.h

Lines changed: 31 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -258,26 +258,28 @@ class Evaluator {
258258
void registerRequestFunctions(Zone zone,
259259
ArrayRef<AbstractRequestFunction *> functions);
260260

261-
/// Evaluate the given request and produce its result,
262-
/// consulting/populating the cache as required.
263-
template<typename Request>
261+
/// Retrieve the result produced by evaluating a request that can
262+
/// be cached.
263+
template<typename Request,
264+
typename std::enable_if<Request::isEverCached>::type * = nullptr>
264265
llvm::Expected<typename Request::OutputType>
265266
operator()(const Request &request) {
266-
// Check for a cycle.
267-
if (checkDependency(getCanonicalRequest(request))) {
268-
return llvm::Error(
269-
llvm::make_unique<CyclicalRequestError<Request>>(request, *this));
270-
}
267+
// The request can be cached, but check a predicate to determine
268+
// whether this particular instance is cached. This allows more
269+
// fine-grained control over which instances get cache.
270+
if (request.isCached())
271+
return getResultCached(request);
271272

272-
// Make sure we remove this from the set of active requests once we're
273-
// done.
274-
SWIFT_DEFER {
275-
assert(activeRequests.back().castTo<Request>() == request);
276-
activeRequests.pop_back();
277-
};
273+
return getResultUncached(request);
274+
}
278275

279-
// Get the result.
280-
return getResult(request);
276+
/// Retrieve the result produced by evaluating a request that
277+
/// will never be cached.
278+
template<typename Request,
279+
typename std::enable_if<!Request::isEverCached>::type * = nullptr>
280+
llvm::Expected<typename Request::OutputType>
281+
operator()(const Request &request) {
282+
return getResultUncached(request);
281283
}
282284

283285
/// Evaluate a set of requests and return their results as a tuple.
@@ -345,34 +347,23 @@ class Evaluator {
345347
/// request to the \c activeRequests stack.
346348
bool checkDependency(const AnyRequest &request);
347349

348-
/// Retrieve the result produced by evaluating a request that can
349-
/// be cached.
350-
template<typename Request,
351-
typename std::enable_if<Request::isEverCached>::type * = nullptr>
352-
llvm::Expected<typename Request::OutputType>
353-
getResult(const Request &request) {
354-
// The request can be cached, but check a predicate to determine
355-
// whether this particular instance is cached. This allows more
356-
// fine-grained control over which instances get cache.
357-
if (request.isCached())
358-
return getResultCached(request);
359-
360-
return getResultUncached(request);
361-
}
362-
363-
/// Retrieve the result produced by evaluating a request that
364-
/// will never be cached.
365-
template<typename Request,
366-
typename std::enable_if<!Request::isEverCached>::type * = nullptr>
367-
llvm::Expected<typename Request::OutputType>
368-
getResult(const Request &request) {
369-
return getResultUncached(request);
370-
}
371-
372350
/// Produce the result of the request without caching.
373351
template<typename Request>
374352
llvm::Expected<typename Request::OutputType>
375353
getResultUncached(const Request &request) {
354+
// Check for a cycle.
355+
if (checkDependency(getCanonicalRequest(request))) {
356+
return llvm::Error(
357+
llvm::make_unique<CyclicalRequestError<Request>>(request, *this));
358+
}
359+
360+
// Make sure we remove this from the set of active requests once we're
361+
// done.
362+
SWIFT_DEFER {
363+
assert(activeRequests.back().castTo<Request>() == request);
364+
activeRequests.pop_back();
365+
};
366+
376367
// Clear out the dependencies on this request; we're going to recompute
377368
// them now anyway.
378369
if (buildDependencyGraph)

0 commit comments

Comments
 (0)