[PATCH] D33493: Speed up preamble loading

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.


================
Comment at: lib/Frontend/ASTUnit.cpp:1152
+  else
+    SrcLocCache.clear();
 
----------------
yvvan wrote:
> arphaman wrote:
> > Why is `clear` in an `else` here? We always create a new `SourceManager` in this function, so the previously cached locations will be invalid, so shouldn't we always clear the cache before `TranslateStoredDiagnostics`?
> When we load diagnostics that means that preamble has not changed. Doesn't that mean that source locations can be reused? What can cause them to become invalid?
> When preamble is invalidated - cache is cleared.
> 
> I can keep cache only during TranslateStoredDiagnostics calls but in that case performance improvement is way less.
> 
> But if you say that current solution is invalid I will do that
You're right actually, we can reuse them. We already do make that assumption that the preamble's source locations can be reused in `checkAndSanitizeDiags`. This code is fine then, sorry about the confusion.

You should mention in the comment for the `SrcLocCache` that we cache only the source locations from the preamble as we can guarantee that they will stay valid when the source manager is re-created.



https://reviews.llvm.org/D33493