Page MenuHomePhabricator

[ASTImporter] Properly delete decls from SavedImportPaths
ClosedPublic

Authored by jarin on Jan 22 2020, 2:31 AM.

Details

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 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.)

Diff Detail

Event Timeline

jarin created this revision.Jan 22 2020, 2:31 AM
Herald added a reviewer: shafik. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
martong accepted this revision.Jan 22 2020, 5:38 AM

Thanks for the fix! Looks good to me!

This revision is now accepted and ready to land.Jan 22 2020, 5:38 AM
jarin added a comment.Jan 22 2020, 6:00 AM

Thanks for the review!

I am not a committer; could you land the fix for me?

I can land it, seems to pass all LLDB tests too.

This revision was automatically updated to reflect the committed changes.