Sema: Move the availability macros cache to the ASTContext #76794
Conversation
The availability macros definitions are parsed from the command line and stored in a cache. The cache was in the Parser, which would have it be computed for each file using availability macros. Let's move it to the ASTContext instead where it can generally be computed once per invocation and used across the module. rdar://134797088
xymus
requested review from
hborla,
slavapestov,
xedin,
ahoppen,
bnbarham,
CodaFi,
DougGregor,
hamishknight and
rintaro
as code owners
|
@swift-ci Please smoke test |
| SmallVector<AvailabilitySpec *, 4>> VersionEntry; | ||
|
|
||
| bool WasParsed = false; | ||
| llvm::DenseMap<StringRef, VersionEntry> Impl; |
There was a problem hiding this comment.
Should this be a StringMap rather than depend on the StringRefs living forever?
There was a problem hiding this comment.
I think that's safe currently. The StringRefs should point to a buffer attached to the SourceManager of the ASTContext, so both the cache and the sources should be freed together. Does this sound right?
There was a problem hiding this comment.
Currently one of these buffers is allocated for each -define-availability argument. I suspect this doesn't help with the general performances of the findBuffer services on wide uses of availability macros, we could look into deregistering these small buffers after parsing and copy all these strings.
| @@ -426,6 +426,9 @@ struct ASTContext::Implementation { | |||
| /// Singleton used to cache the import graph. | |||
| swift::namelookup::ImportCache TheImportCache; | |||
|
|
|||
| /// Cache of availability macros parsed from the command line. | |||
| AvailabilityMacroMap TheAvailabilityMacroCache; | |||
There was a problem hiding this comment.
Could we use the request-evaluator here rather than add the cache directly to the ASTContext?
There was a problem hiding this comment.
The request input here would be the compiler arguments. Since it's used at the parser level we can't rely on them being attached to the module as I would usually do. Would you know what to define as inputs for a request in this case?
There was a problem hiding this comment.
Other requests seem to use the ASTContext itself. I'll see to use that.
…sing The availability macro definitions from the command line are copied into source buffers for parsing. Let's register all of these buffers in one go, before starting to parse any of them. Parsing triggers using `findBuffer` services which sorts all buffers. The new logic should ensure this sorting happens only once for availability macros.
|
@swift-ci Please smoke test |
|
Merging with 2 different optimizations. I'll put up a PR to move the cache to a request next as this optimization is time sensitive. |
The availability macros definitions are parsed from the command line and stored in a cache. The cache was in the Parser, which would have it be computed for each file using availability attributes. Let's move it to the ASTContext instead where it can generally be computed once per invocation and used across the module, speeding up the compilation process.
Also optimize how we register the buffer holding the macro definitions. Registering all of them before starting to parse their content avoids sorting them repeatedly which caused a noticeable slowdown on high usage of availability macros.
rdar://134797088