maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[ASTImporter] Properly delete decls from SavedImportPaths 

Summary:
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 7b81c3f879/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.)

Reviewers: martong, teemperor, a.sidorin, shafik

Reviewed By: martong

Subscribers: rnkovacs, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D73166
This commit is contained in:
Jaroslav Sevcik 2020-01-22 15:19:44 +01:00 committed by Raphael Isemann
parent e3b15ed376
commit 4481eefbe8
1 changed files with 2 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
clang/lib/AST/ASTImporter.cpp
View File

@ -8253,7 +8253,7 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) {
// 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);
@ -8286,7 +8286,7 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) {
Imported(FromD, ToD);
updateFlags(FromD, ToD);
SavedImportPaths[FromD].clear();
SavedImportPaths.erase(FromD);
return ToDOrErr;
}