[Sema] Avoid repeated hash lookups (NFC)

[kazutakahirata]

GlobalMethodPool, the type of MethodPool, is a type wrapping DenseMap
and exposes only a subset of the DenseMap methods.

This patch adds operator[] to GlobalMethodPool so that we can avoid
repeated hash lookups. I don't bother using references or rvalue
references in operator[] because Selector, the key type, is small and
trivially copyable. Note that Selector is a class that wraps a
PointerUnion.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Kazu Hirata (kazutakahirata)

Changes

GlobalMethodPool, the type of MethodPool, is a type wrapping DenseMap
and exposes only a subset of the DenseMap methods.

This patch adds operator[] to GlobalMethodPool so that we can avoid
repeated hash lookups. I don't bother using references or rvalue
references in operator[] because Selector, the key type, is small and
trivially copyable. Note that Selector is a class that wraps a
PointerUnion.

---

Full diff: https://github.com/llvm/llvm-project/pull/109375.diff

2 Files Affected:

• (modified) clang/include/clang/Sema/SemaObjC.h (+1)
• (modified) clang/lib/Sema/SemaDeclObjC.cpp (+2-7)

diff --git a/clang/include/clang/Sema/SemaObjC.h b/clang/include/clang/Sema/SemaObjC.h
index 213c37b5091fe0..b868e9e7cd0aa9 100644
--- a/clang/include/clang/Sema/SemaObjC.h
+++ b/clang/include/clang/Sema/SemaObjC.h
@@ -218,6 +218,7 @@ class SemaObjC : public SemaBase {
     std::pair<iterator, bool> insert(std::pair<Selector, Lists> &&Val) {
       return Methods.insert(Val);
     }
+    Lists &operator[](Selector Key) { return Methods[Key]; }
     int count(Selector Sel) const { return Methods.count(Sel); }
     bool empty() const { return Methods.empty(); }
 
diff --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp
index 807453400abdd0..87be43b13813d3 100644
--- a/clang/lib/Sema/SemaDeclObjC.cpp
+++ b/clang/lib/Sema/SemaDeclObjC.cpp
@@ -3441,16 +3441,11 @@ void SemaObjC::AddMethodToGlobalPool(ObjCMethodDecl *Method, bool impl,
   if (SemaRef.ExternalSource)
     ReadMethodPool(Method->getSelector());
 
-  GlobalMethodPool::iterator Pos = MethodPool.find(Method->getSelector());
-  if (Pos == MethodPool.end())
-    Pos = MethodPool
-              .insert(std::make_pair(Method->getSelector(),
-                                     GlobalMethodPool::Lists()))
-              .first;
+  auto &Lists = MethodPool[Method->getSelector()];
 
   Method->setDefined(impl);
 
-  ObjCMethodList &Entry = instance ? Pos->second.first : Pos->second.second;
+  ObjCMethodList &Entry = instance ? Lists.first : Lists.second;
   addMethodToGlobalList(&Entry, Method);
 }
 

[nikic]

Hm, why does this wrapper exist at all? It looks like a trivial wrapper that doesn't add or change any DenseMap functionality.

[kazutakahirata]

Hm, why does this wrapper exist at all? It looks like a trivial wrapper that doesn't add or change any DenseMap functionality.

I was wondering about the same thing. Let me get rid of the wrapper in a subsequent patch.