[PATCH] Speed up modules compilation.

[Manuel Klimek]

In http://reviews.llvm.org/D9498#175201, @rsmith wrote:

> Essentially this seems fine. It'd be better if `visitDepthFirst` didn't actually walk the the entire module graph in the case where it's been told to `SkipChildren`, but we can leave that improvement for a later change.


I'm not sure how that would be possible due to imports building a DAG.
The trade-off seems to be: either we remember the knowledge that all transitive deps do not need to be visited, or we might have to re-calculate that information when we come in via a different edge. My experiments indicate that the latter is common enough to offset the cost of the former.


================
Comment at: include/clang/Serialization/ModuleManager.h:295-296
@@ -283,4 +294,4 @@
   /// which will be passed along to the user.
-  void visitDepthFirst(bool (*Visitor)(ModuleFile &M, bool Preorder, 
-                                       void *UserData), 
+  void visitDepthFirst(bool (*PostorderVisitor)(ModuleFile &M, void *UserData),
+                       DFSPreorderControl (*PreorderVisitor)(ModuleFile &M, void *UserData),
                        void *UserData);
----------------
rsmith wrote:
> These parameters seem backwards to me (on each node, we'll call the preorder visitor before the postorder one, so it would seem more natural for the preorder one to be written first in a call to this function).
... says the person whose trees grow upside-down ;) Done.

================
Comment at: lib/Serialization/ASTReaderDecl.cpp:3363
@@ +3362,3 @@
+
+    bool needsToVisitChildren(ModuleFile &M, GlobalDeclID GlobalID) {
+      DeclID ID = Reader.mapGlobalIDToModuleFileGlobalID(M, GlobalID);
----------------
rsmith wrote:
> `Children` here is ambiguous, and entirely backwards from the way I think of the imports graph (where the roots import nothing and the leaves are not imported by anything). Calling this `Imports` would remove the ambiguity (likewise in the `DFSPreorderControl` enum).
Note that there is precedence in the ModuleManager.h of calling the imports "children" :P
Your trees grow the wrong way (granted, this isn't *really* a tree).
On the other hand, "imports" is clearly the better name, so Done :D

================
Comment at: lib/Serialization/ASTReaderDecl.cpp:3376-3378
@@ +3375,5 @@
+      }
+      unsigned Offset = Result->Offset;
+      unsigned N = M.RedeclarationChains[Offset];
+      return N != 0;
+    }
----------------
rsmith wrote:
> A comment explaining what's going on here ("we don't need to visit a module or any of its imports if we've already deserialized the redecls from this module") would help.
Done.

http://reviews.llvm.org/D9498

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/