jarin created this revision.
jarin added a reviewer: martong.
jarin added projects: LLDB, clang.
Herald added subscribers: cfe-commits, teemperor, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
jarin edited the summary of this revision.
This is a work in progress patch for discussion. The goal is to improve
performance of LLDB's expression evaluator.
During my investigations, I noticed that the AST importer is very eager to
import classes/structs that were already completed on the 'From' side, even if
they are not needed. This is best illustrated with an example:
struct C0 { int x = 0; };
struct C1 { int x = 1; C0* c0 = 0; };
struct C2 { int x = 2; C1* c1 = 0; };
int main() {
C0 c0;
C1 c1;
C2 c2;
return 0; // break here
}
When we evaluate “c2.x” in LLDB, AST importer completes and imports only class
C2. This is working as intended. Similarly, evaluating “c1.x” imports just C1
and “c0.x” imports C0. However, if we evaluate “c2.x” after evaluating “c1.x”
and “c0.x”, the importer suddenly imports both C1 and C0 (in addition to C2).
See a log from a lldb session at the end of this email for illustration.
I believe the culprit here is the following code at the end of the
ASTNodeImporter::VisitRecordDecl method:
if (D->isCompleteDefinition())
if (Error Err = ImportDefinition(D, D2, IDK_Default))
return std::move(Err);
This will import a definition of a class from LLDB if LLDB already happens to
have a complete definition from before. For large programs, this can lead to
importing very large chunks of ASTs even if they are not needed. I have tried
to remove the code above from the AST importer and test performance on several
expressions in an Unreal engine sample - preliminary results show this cuts
down evaluation time by roughly 50%.
This is work in progress, couple of lldb tests are failing (but hopefully
fixable). What would the experts here think? Is this a plausible direction?
—— lldb session illustrating the unnecessary imports —-
This shows that evaluation of “c2.x” after evaluation “c1.x” and “c0.x” calls
to LayoutRecordType for C2, C1 and C0.
$ lldb a.out
(lldb) b h.cc:10
Breakpoint 1: where = a.out`main + 44 at h.cc:10:3, address = ...
(lldb) r
... Process stopped ...
(lldb) log enable lldb expr
(lldb) p c2.x
...
LayoutRecordType[6] ... for (RecordDecl*)0x... [name = 'C2']
...
(lldb) p c1.x
...
LayoutRecordType[7] ... for (RecordDecl*)0x... [name = 'C1']
...
(lldb) p c0.x
...
LayoutRecordType[8] ... for (RecordDecl*)0x... [name = 'C0']
...
(lldb) p c2.x
...
LayoutRecordType[9] ... for (RecordDecl*)0x... [name = 'C2']
LayoutRecordType[10] ... for (RecordDecl*)0x... [name = 'C1']
LayoutRecordType[11] ... for (RecordDecl*)0x... [name = 'C0']
...
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D69933
Files:
clang/lib/AST/ASTImporter.cpp
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -2801,7 +2801,7 @@
if (D->isAnonymousStructOrUnion())
D2->setAnonymousStructOrUnion(true);
- if (D->isCompleteDefinition())
+ if (!Importer.isMinimalImport() && D->isCompleteDefinition())
if (Error Err = ImportDefinition(D, D2, IDK_Default))
return std::move(Err);
@@ -3440,6 +3440,9 @@
if (ToInitializer)
ToField->setInClassInitializer(ToInitializer);
ToField->setImplicit(D->isImplicit());
+ if (CXXRecordDecl *FieldType = D->getType()->getAsCXXRecordDecl())
+ if (Error Err = ImportDefinitionIfNeeded(FieldType))
+ return std::move(Err);
LexicalDC->addDeclInternal(ToField);
return ToField;
}
@@ -5323,7 +5326,7 @@
D2->setTemplateSpecializationKind(D->getTemplateSpecializationKind());
- if (D->isCompleteDefinition())
+ if (!Importer.isMinimalImport() && D->isCompleteDefinition())
if (Error Err = ImportDefinition(D, D2))
return std::move(Err);
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -2801,7 +2801,7 @@
if (D->isAnonymousStructOrUnion())
D2->setAnonymousStructOrUnion(true);
- if (D->isCompleteDefinition())
+ if (!Importer.isMinimalImport() && D->isCompleteDefinition())
if (Error Err = ImportDefinition(D, D2, IDK_Default))
return std::move(Err);
@@ -3440,6 +3440,9 @@
if (ToInitializer)
ToField->setInClassInitializer(ToInitializer);
ToField->setImplicit(D->isImplicit());
+ if (CXXRecordDecl *FieldType = D->getType()->getAsCXXRecordDecl())
+ if (Error Err = ImportDefinitionIfNeeded(FieldType))
+ return std::move(Err);
LexicalDC->addDeclInternal(ToField);
return ToField;
}
@@ -5323,7 +5326,7 @@
D2->setTemplateSpecializationKind(D->getTemplateSpecializationKind());
- if (D->isCompleteDefinition())
+ if (!Importer.isMinimalImport() && D->isCompleteDefinition())
if (Error Err = ImportDefinition(D, D2))
return std::move(Err);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
