[LLDB][NativePDB] Improve ParseDeclsForContext time.

[zequanwu]

Summary

1. When we evaluating an expression multiple times and the searching scope is translation unit, ParseDeclsForContext iterates the type info and symbol info multiple times, though only the debug info is parsed once. Using llvm::call_once to make it only iterating and parsing once.

2. When evaluating an expression with identifier whose parent scope is a namespace, ParseDeclsForContext needs to search the entire type info to complete those records whose name is prefixed with the namespace's name and the entire symbol info to to parse functions and non-local variables. Caching parsed namespaces to avoid unnecessary searching.

[zequanwu]

update.

[labath]

Is this purely a performance optimization, or does it have some impact on operation (its correctness) as well? E.g. does it prevent duplicate construction of some types or something like that?

[zequanwu]

In D136006#3864144, @labath wrote:

Is this purely a performance optimization, or does it have some impact on operation (its correctness) as well? E.g. does it prevent duplicate construction of some types or something like that?

It's purely a performance optimization. Avoid doing some unnecessary work.

[labath]

Okay, sounds good then.

lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp

1413–1414

I have a feeling this is still doing more work than it would be necessary. I haven't checked, but I'd expect that here it should be sufficient to parse only the top level namespace names (not their contents), and create forward declarations for the classes in the global namespace. I suspect this is doing much more than that.
(Of course, if PDB makes it hard to parse just this information, then it might actually be better to parse everything -- I just don't know)

[zequanwu]

Don't try to complete types in PdbAstBuilder::ParseDeclsForContext.

lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp

1413–1414

Yeah, we can do that.
Update to don't try to complete types at all in PdbAstBuilder::ParseDeclsForContext. For a chrome crash report I'm looking at, the time for evaluating a unknown identifier(which will have search scope being a TU) drops from 287s -> 160s.

[labath]

lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp

1413–1414

cool

lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h

 //===-- PdbAstBuilder.h -----------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_SOURCE_PLUGINS_SYMBOLFILE_NATIVEPDB_PDBASTBUILDER_H
 #define LLDB_SOURCE_PLUGINS_SYMBOLFILE_NATIVEPDB_PDBASTBUILDER_H
 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Threading.h"
 
 #include "Plugins/ExpressionParser/Clang/ClangASTImporter.h"
 
 #include "PdbIndex.h"
 #include "PdbSymUid.h"
 
 namespace clang {
 class TagDecl;
clang::NamespaceDecl *GetOrCreateNamespaceDecl(const char *name,
                                                  clang::DeclContext &context);
   clang::FunctionDecl *CreateFunctionDeclFromId(PdbTypeSymId func_tid,
                                                 PdbCompilandSymId func_sid);
   clang::FunctionDecl *
   CreateFunctionDecl(PdbCompilandSymId func_id, llvm::StringRef func_name,
                      TypeIndex func_ti, CompilerType func_ct,
                      uint32_t param_count, clang::StorageClass func_storage,
                      bool is_inline, clang::DeclContext *parent);
-  void ParseAllNamespacesPlusChildrenOf(llvm::Optional<llvm::StringRef> parent);
+  void ParseNamespace(clang::DeclContext &parent);
+  void ParseAllTypes();
+  void ParseAllFunctionsAndNonLocalVars();
   void ParseDeclsForSimpleContext(clang::DeclContext &context);
   void ParseBlockChildren(PdbCompilandSymId block_id);
 
   std::pair<clang::DeclContext *, std::string>
   CreateDeclInfoForType(const llvm::codeview::TagRecord &record, TypeIndex ti);
   std::pair<clang::DeclContext *, std::string>
   CreateDeclInfoForUndecoratedName(llvm::StringRef uname);
   clang::QualType CreateSimpleType(TypeIndex ti);
 
   TypeSystemClang &m_clang;
 
   ClangASTImporter m_importer;
-
+  llvm::once_flag m_parse_functions_and_non_local_vars;
+  llvm::once_flag m_parse_all_types;
   llvm::DenseMap<clang::Decl *, DeclStatus> m_decl_to_status;
   llvm::DenseMap<lldb::user_id_t, clang::Decl *> m_uid_to_decl;
   llvm::DenseMap<lldb::user_id_t, clang::QualType> m_uid_to_type;
 
   // From class/struct's opaque_compiler_type_t to a set containing the pairs of
   // method's name and CompilerType.
   llvm::DenseMap<lldb::opaque_compiler_type_t,
                  llvm::SmallSet<std::pair<llvm::StringRef, CompilerType>, 8>>
       m_cxx_record_map;
+  llvm::DenseSet<clang::NamespaceDecl *> m_parsed_namespaces;
 };
 
 } // namespace npdb
 } // namespace lldb_private
 
 #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_NATIVEPDB_PDBASTBUILDER_H

lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp

 
 #include "Plugins/ExpressionParser/Clang/ClangASTMetadata.h"
 #include "Plugins/ExpressionParser/Clang/ClangUtil.h"
 #include "Plugins/Language/CPlusPlus/MSVCUndecoratedNameParser.h"
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/LLDBAssert.h"
-
 #include "PdbUtil.h"
 #include "UdtRecordCompleter.h"
 #include "SymbolFileNativePDB.h"
 
 using namespace lldb_private;
 using namespace lldb_private::npdb;
 using namespace llvm::codeview;
 using namespace llvm::pdb;

 static bool isFunctionDecl(clang::DeclContext &context) {
   return llvm::isa<clang::FunctionDecl>(&context);
 }
 
 static bool isBlockDecl(clang::DeclContext &context) {
   return llvm::isa<clang::BlockDecl>(&context);
 }
 
-void PdbAstBuilder::ParseAllNamespacesPlusChildrenOf(
-    llvm::Optional<llvm::StringRef> parent) {
+void PdbAstBuilder::ParseNamespace(clang::DeclContext &context) {
+  clang::NamespaceDecl *ns = llvm::dyn_cast<clang::NamespaceDecl>(&context);
+  if (m_parsed_namespaces.contains(ns))
+    return;
+  std::string qname = ns->getQualifiedNameAsString();
   SymbolFileNativePDB *pdb = static_cast<SymbolFileNativePDB *>(
       m_clang.GetSymbolFile()->GetBackingSymbolFile());
   PdbIndex &index = pdb->GetIndex();
   TypeIndex ti{index.tpi().TypeIndexBegin()};
   for (const CVType &cvt : index.tpi().typeArray()) {
     PdbTypeSymId tid{ti};
     ++ti;
 
     if (!IsTagRecord(cvt))
       continue;
 
     CVTagRecord tag = CVTagRecord::create(cvt);
 
-    if (!parent) {
-      clang::QualType qt = GetOrCreateType(tid);
-      CompleteType(qt);
-      continue;
-    }
-
     // Call CreateDeclInfoForType unconditionally so that the namespace info
     // gets created.  But only call CreateRecordType if the namespace name
     // matches.
     clang::DeclContext *context = nullptr;
     std::string uname;
     std::tie(context, uname) = CreateDeclInfoForType(tag.asTag(), tid.index);
     if (!context || !context->isNamespace())
       continue;
 
     clang::NamespaceDecl *ns = llvm::cast<clang::NamespaceDecl>(context);
-    std::string actual_ns = ns->getQualifiedNameAsString();
-    if (llvm::StringRef(actual_ns).startswith(*parent)) {
+    llvm::StringRef ns_name = ns->getName();
+    if (ns_name.startswith(qname)) {
+      ns_name = ns_name.drop_front(qname.size());
+      if (ns_name.startswith("::")) {
         clang::QualType qt = GetOrCreateType(tid);
         CompleteType(qt);
+      }
+    }
+  }
+  ParseAllFunctionsAndNonLocalVars();
+  m_parsed_namespaces.insert(ns);
+}
+
+void PdbAstBuilder::ParseAllTypes() {
+  llvm::call_once(m_parse_all_types, [this]() {
+    SymbolFileNativePDB *pdb = static_cast<SymbolFileNativePDB *>(
+        m_clang.GetSymbolFile()->GetBackingSymbolFile());
+    PdbIndex &index = pdb->GetIndex();
+    TypeIndex ti{index.tpi().TypeIndexBegin()};
+    for (const CVType &cvt : index.tpi().typeArray()) {
+      PdbTypeSymId tid{ti};
+      ++ti;
+
+      if (!IsTagRecord(cvt))
         continue;
+
+      clang::QualType qt = GetOrCreateType(tid);
+      CompleteType(qt);
     }
-  }
+  });
 
+void PdbAstBuilder::ParseAllFunctionsAndNonLocalVars() {
+  llvm::call_once(m_parse_functions_and_non_local_vars, [this]() {
+    SymbolFileNativePDB *pdb = static_cast<SymbolFileNativePDB *>(
+        m_clang.GetSymbolFile()->GetBackingSymbolFile());
+    PdbIndex &index = pdb->GetIndex();
     uint32_t module_count = index.dbi().modules().getModuleCount();
     for (uint16_t modi = 0; modi < module_count; ++modi) {
       CompilandIndexItem &cii = index.compilands().GetOrCreateCompiland(modi);
       const CVSymbolArray &symbols = cii.m_debug_stream.getSymbolArray();
       auto iter = symbols.begin();
       while (iter != symbols.end()) {
         PdbCompilandSymId sym_id{modi, iter.offset()};
 
         switch (iter->kind()) {
         case S_GPROC32:
         case S_LPROC32:
           GetOrCreateFunctionDecl(sym_id);
           iter = symbols.at(getScopeEndOffset(*iter));
           break;
         case S_GDATA32:
         case S_GTHREAD32:
         case S_LDATA32:
         case S_LTHREAD32:
           GetOrCreateVariableDecl(PdbCompilandSymId(modi, 0), sym_id);
           ++iter;
           break;
         default:
           ++iter;
           continue;
         }
       }
     }
+  });
 }
 
 static CVSymbolArray skipFunctionParameters(clang::Decl &decl,
                                             const CVSymbolArray &symbols) {
   clang::FunctionDecl *func_decl = llvm::dyn_cast<clang::FunctionDecl>(&decl);
   if (!func_decl)
     return symbols;
   unsigned int params = func_decl->getNumParams();

 void PdbAstBuilder::ParseDeclsForContext(clang::DeclContext &context) {
   // Namespaces aren't explicitly represented in the debug info, and the only
   // way to parse them is to parse all type info, demangling every single type
   // and trying to reconstruct the DeclContext hierarchy this way.  Since this
   // is an expensive operation, we have to special case it so that we do other
   // work (such as parsing the items that appear within the namespaces) at the
   // same time.
   if (context.isTranslationUnit()) {
-    ParseAllNamespacesPlusChildrenOf(llvm::None);
+    ParseAllTypes();
+    ParseAllFunctionsAndNonLocalVars();
     return;
   }
 
   if (context.isNamespace()) {
-    clang::NamespaceDecl &ns = *llvm::dyn_cast<clang::NamespaceDecl>(&context);
-    std::string qname = ns.getQualifiedNameAsString();
-    ParseAllNamespacesPlusChildrenOf(llvm::StringRef{qname});
+    ParseNamespace(context);
     return;
   }
 
   if (isTagDecl(context) || isFunctionDecl(context) || isBlockDecl(context)) {
     ParseDeclsForSimpleContext(context);
     return;
   }
 }