jarin created this revision.
jarin added reviewers: martong, teemperor.
Herald added subscribers: cfe-commits, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: clang.
We see a significant regression (~40% slower on large codebases) in expression
evaluation after https://reviews.llvm.org/rL364771. A sampling profile shows
the extra time is spent in SavedImportPathsTy::operator[] when called from
ASTImporter::Import. I believe this is because ASTImporter::Import adds an
element to the SavedImportPaths map for each decl unconditionally (see
https://github.com/llvm/llvm-project/blob/7b81c3f8793d30a4285095a9b67dcfca2117916c/clang/lib/AST/ASTImporter.cpp#L8256).
To fix this, we call SavedImportPathsTy::erase on the declaration rather than
clearing its value vector. That way we do not accidentally introduce new empty
elements. (With this patch the performance is restored, and we do not see
SavedImportPathsTy::operator[] in the profile anymore.)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D73166
Files:
clang/lib/AST/ASTImporter.cpp
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -8241,7 +8241,7 @@
// FIXME Should we remove these Decls from the LookupTable,
// and from ImportedFromDecls?
}
- SavedImportPaths[FromD].clear();
+ SavedImportPaths.erase(FromD);
// Do not return ToDOrErr, error was taken out of it.
return make_error<ImportError>(ErrOut);
@@ -8274,7 +8274,7 @@
Imported(FromD, ToD);
updateFlags(FromD, ToD);
- SavedImportPaths[FromD].clear();
+ SavedImportPaths.erase(FromD);
return ToDOrErr;
}
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -8241,7 +8241,7 @@
// FIXME Should we remove these Decls from the LookupTable,
// and from ImportedFromDecls?
}
- SavedImportPaths[FromD].clear();
+ SavedImportPaths.erase(FromD);
// Do not return ToDOrErr, error was taken out of it.
return make_error<ImportError>(ErrOut);
@@ -8274,7 +8274,7 @@
Imported(FromD, ToD);
updateFlags(FromD, ToD);
- SavedImportPaths[FromD].clear();
+ SavedImportPaths.erase(FromD);
return ToDOrErr;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
