[clang] 0a35cc4 - [clang][objc] Speed up populating the global method pool from modules.
Volodymyr Sapsai via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 3 17:11:44 PDT 2021
Author: Volodymyr Sapsai
Date: 2021-11-03T17:11:14-07:00
New Revision: 0a35cc40b881a35c6a7a748f5fdefac4cafc33ce
URL: https://github.com/llvm/llvm-project/commit/0a35cc40b881a35c6a7a748f5fdefac4cafc33ce
DIFF: https://github.com/llvm/llvm-project/commit/0a35cc40b881a35c6a7a748f5fdefac4cafc33ce.diff
LOG: [clang][objc] Speed up populating the global method pool from modules.
For each selector encountered in the source code, we need to load
selectors from the imported modules and check that we are calling a
selector with compatible types.
At the moment, for each module we are storing methods declared in the
headers belonging to this module and methods from the transitive closure
of imported modules. When a module is imported by a few other modules,
methods from the shared module are duplicated in each importer. As the
result, we can end up with lots of identical methods that we try to add
to the global method pool. Doing this duplicate work is useless and
relatively expensive.
Avoid processing duplicate methods by storing in each module only its
own methods and not storing methods from dependencies. Collect methods
from dependencies by walking the graph of module dependencies.
The issue was discovered and reported by Richard Howell. He has done the
hard work for this fix as he has investigated and provided a detailed
explanation of the performance problem.
Differential Revision: https://reviews.llvm.org/D110123
Added:
clang/test/Modules/method_pool_transitive.m
Modified:
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/Modules/lookup.m
Removed:
################################################################################
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 0f180d821ed7f..07a0f00aa5357 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -8151,13 +8151,16 @@ namespace serialization {
if (Reader.DeserializationListener)
Reader.DeserializationListener->SelectorRead(Data.ID, Sel);
- InstanceMethods.append(Data.Instance.begin(), Data.Instance.end());
- FactoryMethods.append(Data.Factory.begin(), Data.Factory.end());
+ // Append methods in the reverse order, so that later we can process them
+ // in the order they appear in the source code by iterating through
+ // the vector in the reverse order.
+ InstanceMethods.append(Data.Instance.rbegin(), Data.Instance.rend());
+ FactoryMethods.append(Data.Factory.rbegin(), Data.Factory.rend());
InstanceBits = Data.InstanceBits;
FactoryBits = Data.FactoryBits;
InstanceHasMoreThanOneDecl = Data.InstanceHasMoreThanOneDecl;
FactoryHasMoreThanOneDecl = Data.FactoryHasMoreThanOneDecl;
- return true;
+ return false;
}
/// Retrieve the instance methods found by this visitor.
@@ -8186,9 +8189,8 @@ namespace serialization {
/// Add the given set of methods to the method list.
static void addMethodsToPool(Sema &S, ArrayRef<ObjCMethodDecl *> Methods,
ObjCMethodList &List) {
- for (unsigned I = 0, N = Methods.size(); I != N; ++I) {
- S.addMethodToGlobalList(&List, Methods[I]);
- }
+ for (auto I = Methods.rbegin(), E = Methods.rend(); I != E; ++I)
+ S.addMethodToGlobalList(&List, *I);
}
void ASTReader::ReadMethodPool(Selector Sel) {
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 7c500f30e271e..3e0e5e9c8f752 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -3045,11 +3045,11 @@ class ASTMethodPoolTrait {
unsigned DataLen = 4 + 2 + 2; // 2 bytes for each of the method counts
for (const ObjCMethodList *Method = &Methods.Instance; Method;
Method = Method->getNext())
- if (Method->getMethod())
+ if (ShouldWriteMethodListNode(Method))
DataLen += 4;
for (const ObjCMethodList *Method = &Methods.Factory; Method;
Method = Method->getNext())
- if (Method->getMethod())
+ if (ShouldWriteMethodListNode(Method))
DataLen += 4;
return emitULEBKeyDataLength(KeyLen, DataLen, Out);
}
@@ -3080,13 +3080,13 @@ class ASTMethodPoolTrait {
unsigned NumInstanceMethods = 0;
for (const ObjCMethodList *Method = &Methods.Instance; Method;
Method = Method->getNext())
- if (Method->getMethod())
+ if (ShouldWriteMethodListNode(Method))
++NumInstanceMethods;
unsigned NumFactoryMethods = 0;
for (const ObjCMethodList *Method = &Methods.Factory; Method;
Method = Method->getNext())
- if (Method->getMethod())
+ if (ShouldWriteMethodListNode(Method))
++NumFactoryMethods;
unsigned InstanceBits = Methods.Instance.getBits();
@@ -3107,15 +3107,20 @@ class ASTMethodPoolTrait {
LE.write<uint16_t>(FullFactoryBits);
for (const ObjCMethodList *Method = &Methods.Instance; Method;
Method = Method->getNext())
- if (Method->getMethod())
+ if (ShouldWriteMethodListNode(Method))
LE.write<uint32_t>(Writer.getDeclID(Method->getMethod()));
for (const ObjCMethodList *Method = &Methods.Factory; Method;
Method = Method->getNext())
- if (Method->getMethod())
+ if (ShouldWriteMethodListNode(Method))
LE.write<uint32_t>(Writer.getDeclID(Method->getMethod()));
assert(Out.tell() - Start == DataLen && "Data length is wrong");
}
+
+private:
+ static bool ShouldWriteMethodListNode(const ObjCMethodList *Node) {
+ return (Node->getMethod() && !Node->getMethod()->isFromASTFile());
+ }
};
} // namespace
@@ -3158,15 +3163,21 @@ void ASTWriter::WriteSelectors(Sema &SemaRef) {
if (Chain && ID < FirstSelectorID) {
// Selector already exists. Did it change?
bool changed = false;
- for (ObjCMethodList *M = &Data.Instance;
- !changed && M && M->getMethod(); M = M->getNext()) {
- if (!M->getMethod()->isFromASTFile())
+ for (ObjCMethodList *M = &Data.Instance; M && M->getMethod();
+ M = M->getNext()) {
+ if (!M->getMethod()->isFromASTFile()) {
changed = true;
+ Data.Instance = *M;
+ break;
+ }
}
- for (ObjCMethodList *M = &Data.Factory; !changed && M && M->getMethod();
+ for (ObjCMethodList *M = &Data.Factory; M && M->getMethod();
M = M->getNext()) {
- if (!M->getMethod()->isFromASTFile())
+ if (!M->getMethod()->isFromASTFile()) {
changed = true;
+ Data.Factory = *M;
+ break;
+ }
}
if (!changed)
continue;
diff --git a/clang/test/Modules/lookup.m b/clang/test/Modules/lookup.m
index b22e41f845942..0e09bfdd7fd95 100644
--- a/clang/test/Modules/lookup.m
+++ b/clang/test/Modules/lookup.m
@@ -10,8 +10,8 @@
void test(id x) {
[x method];
// expected-warning at -1{{multiple methods named 'method' found}}
-// expected-note at Inputs/lookup_left.h:2{{using}}
-// expected-note at Inputs/lookup_right.h:3{{also found}}
+// expected-note at Inputs/lookup_right.h:3{{using}}
+// expected-note at Inputs/lookup_left.h:2{{also found}}
}
// CHECK-PRINT: - (int)method;
diff --git a/clang/test/Modules/method_pool_transitive.m b/clang/test/Modules/method_pool_transitive.m
new file mode 100644
index 0000000000000..40c4330b75009
--- /dev/null
+++ b/clang/test/Modules/method_pool_transitive.m
@@ -0,0 +1,40 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -Wobjc-multiple-method-names -fsyntax-only -fmodules-cache-path=%t/modules.cache -fmodules -fimplicit-module-maps -F %t/Frameworks %t/test.m -verify
+
+// Verify we are handling methods from transitive modules, not just from immediate ones.
+
+//--- Frameworks/Indirect.framework/Headers/Indirect.h
+ at interface NSObject
+ at end
+
+ at interface Indirect : NSObject
+- (int)method;
+ at end
+
+//--- Frameworks/Indirect.framework/Modules/module.modulemap
+framework module Indirect {
+ header "Indirect.h"
+ export *
+}
+
+//--- Frameworks/Immediate.framework/Headers/Immediate.h
+#import <Indirect/Indirect.h>
+ at interface Immediate : NSObject
+- (void)method;
+ at end
+
+//--- Frameworks/Immediate.framework/Modules/module.modulemap
+framework module Immediate {
+ header "Immediate.h"
+ export *
+}
+
+//--- test.m
+#import <Immediate/Immediate.h>
+
+void test(id obj) {
+ [obj method]; // expected-warning{{multiple methods named 'method' found}}
+ // expected-note at Frameworks/Indirect.framework/Headers/Indirect.h:5{{using}}
+ // expected-note at Frameworks/Immediate.framework/Headers/Immediate.h:3{{also found}}
+}
More information about the cfe-commits
mailing list