[cfe-commits] [PATCH] More Abbreviations for PCH Serialization

[Douglas Gregor]

On Jun 1, 2011, at 2:35 PM, Jonathan Turner wrote:

> The attached patch adds five more abbreviations to PCH serialization, with the intention of decreasing PCH sizes.  
> 
> With this patch, PCH binary sizes decrease ~1.2% for Objective-C headers like Cocoa and WebKit and ~2.6% for C++ headers like string and iostream.  
> 
> This patch also includes a minor fix for the ASTWriter so that more AST node types can be resolved when using tools like llvm-bcanalyzer.


Generally looks good, with a few comments:

Index: include/clang/Serialization/ASTWriter.h
===================================================================
--- include/clang/Serialization/ASTWriter.h	(revision 132422)
+++ include/clang/Serialization/ASTWriter.h	(working copy)
@@ -279,6 +279,9 @@
   /// file.
   unsigned NumVisibleDeclContexts;
 
+  /// \brief The number of allowed abbreviations in bits
+  unsigned NumAllowedAbbrevsSize;
+

This should be a namespace-level constant named something like "DeclTypeBlockCodeSize", rather than a member of ASTWriter. It isn't going to change.

Index: lib/Serialization/ASTWriterStmt.cpp
===================================================================
--- lib/Serialization/ASTWriterStmt.cpp	(revision 132422)
+++ lib/Serialization/ASTWriterStmt.cpp	(working copy)
@@ -30,6 +30,7 @@
 
   public:
     serialization::StmtCode Code;
+    unsigned AbbrevToUse;
 
     ASTStmtWriter(ASTWriter &Writer, ASTWriter::RecordData &Record)
       : Writer(Writer), Record(Record) { }
@@ -392,6 +393,13 @@
     Record.push_back(NumTemplateArgs);
   }
 
+  DeclarationName::NameKind nk = (E->getDecl()->getDeclName().getNameKind());
+
+  if ((!E->hasExplicitTemplateArgs()) && (!E->hasQualifier()) && (!(E->getDecl() != E->getFoundDecl()))

Why not just use == for this last condition?

+      && (nk == DeclarationName::Identifier || nk == DeclarationName::ObjCZeroArgSelector)) {
+    AbbrevToUse = Writer.getDeclRefExprAbbrev();
+  }
+

Why is the DeclarationName::ObjCZeroArgSelector check here?

@@ -411,6 +419,11 @@
   VisitExpr(E);
   Writer.AddSourceLocation(E->getLocation(), Record);
   Writer.AddAPInt(E->getValue(), Record);
+
+  if (E->getValue().getBitWidth() == 32) {
+    AbbrevToUse = Writer.getIntegerLiteralAbbrev();
+  }
+
   Code = serialization::EXPR_INTEGER_LITERAL;
 }

Are we getting a decent abbreviation percentage for just 32-bit integer literals? It  seems to me like we've have a lot of 'long' literals as well in headers, particularly when we're dealing with the null pointer constant 0L when compiling for a 64-bit platform.

Thanks for working on this!

	- Doug