[PATCH] llvm-symbolizer: speedup symbol lookup

Alexey Samsonov samsonov at google.com
Thu Feb 14 03:19:34 PST 2013


  LGTM


================
Comment at: llvm-symbolizer/LLVMSymbolize.h:92
@@ -83,1 +91,3 @@
+  SymbolMapTy Functions;
+  SymbolMapTy Variables;
 };
----------------
I'd prefer Objects instead of Variables.

================
Comment at: llvm-symbolizer/LLVMSymbolize.cpp:74
@@ +73,3 @@
+    // with same address size. Make sure we choose the correct one.
+    SymbolMapTy& m = SymbolType == SymbolRef::ST_Function ?
+        Functions : Variables;
----------------
Please start variable names with capital letters (or use abbrevs like SymbolDesc SD) and prefer "SymbolMapTy &M" over "SymbolMapTy& M" for consistency.

================
Comment at: llvm-symbolizer/LLVMSymbolize.cpp:84
@@ +83,3 @@
+                                        uint64_t &Size) const {
+  const SymbolMapTy& m = Type == SymbolRef::ST_Function ?
+      Functions : Variables;
----------------
ditto

================
Comment at: llvm-symbolizer/LLVMSymbolize.cpp:90
@@ +89,3 @@
+    return false;
+  Name = it->second.str();
+  Addr = it->first.Addr;
----------------
Here I would be conservative and check that Address indeed lies in [first.Addr, first.AddrEnd) - this is not guaranteed, because your operator< would only work for cases when no two functions/objects intersect.  This should technically be true, but additional checking would be nice.


http://llvm-reviews.chandlerc.com/D407



More information about the llvm-commits mailing list