[PATCH] D52611: [clangd] Add more tracing to index queries. NFC

2018-09-27 Thread Eric Liu via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL343247: [clangd] Add more tracing to index queries. NFC 
(authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D52611

Files:
  clang-tools-extra/trunk/clangd/index/MemIndex.cpp
  clang-tools-extra/trunk/clangd/index/Merge.cpp
  clang-tools-extra/trunk/clangd/index/dex/Dex.cpp

Index: clang-tools-extra/trunk/clangd/index/MemIndex.cpp
===
--- clang-tools-extra/trunk/clangd/index/MemIndex.cpp
+++ clang-tools-extra/trunk/clangd/index/MemIndex.cpp
@@ -11,6 +11,7 @@
 #include "FuzzyMatch.h"
 #include "Logger.h"
 #include "Quality.h"
+#include "Trace.h"
 
 namespace clang {
 namespace clangd {
@@ -28,6 +29,7 @@
 llvm::function_ref Callback) const {
   assert(!StringRef(Req.Query).contains("::") &&
  "There must be no :: in query.");
+  trace::Span Tracer("MemIndex fuzzyFind");
 
   TopN> Top(
   Req.Limit ? *Req.Limit : std::numeric_limits::max());
@@ -47,13 +49,16 @@
   if (Top.push({*Score * quality(*Sym), Sym}))
 More = true; // An element with smallest score was discarded.
   }
-  for (const auto  : std::move(Top).items())
+  auto Results = std::move(Top).items();
+  SPAN_ATTACH(Tracer, "results", static_cast(Results.size()));
+  for (const auto  : Results)
 Callback(*Item.second);
   return More;
 }
 
 void MemIndex::lookup(const LookupRequest ,
   llvm::function_ref Callback) const {
+  trace::Span Tracer("MemIndex lookup");
   for (const auto  : Req.IDs) {
 auto I = Index.find(ID);
 if (I != Index.end())
@@ -63,6 +68,7 @@
 
 void MemIndex::refs(const RefsRequest ,
 llvm::function_ref Callback) const {
+  trace::Span Tracer("MemIndex refs");
   for (const auto  : Req.IDs) {
 auto SymRefs = Refs.find(ReqID);
 if (SymRefs == Refs.end())
Index: clang-tools-extra/trunk/clangd/index/Merge.cpp
===
--- clang-tools-extra/trunk/clangd/index/Merge.cpp
+++ clang-tools-extra/trunk/clangd/index/Merge.cpp
@@ -9,6 +9,7 @@
 
 #include "Merge.h"
 #include "Logger.h"
+#include "Trace.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/raw_ostream.h"
@@ -38,19 +39,31 @@
  //a) if it's not in the dynamic slab, yield it directly
  //b) if it's in the dynamic slab, merge it and yield the result
  //  3) now yield all the dynamic symbols we haven't processed.
+ trace::Span Tracer("MergedIndex fuzzyFind");
  bool More = false; // We'll be incomplete if either source was.
  SymbolSlab::Builder DynB;
- More |= Dynamic->fuzzyFind(Req, [&](const Symbol ) { DynB.insert(S); });
+ unsigned DynamicCount = 0;
+ unsigned StaticCount = 0;
+ unsigned MergedCount = 0;
+ More |= Dynamic->fuzzyFind(Req, [&](const Symbol ) {
+   ++DynamicCount;
+   DynB.insert(S);
+ });
  SymbolSlab Dyn = std::move(DynB).build();
 
  DenseSet SeenDynamicSymbols;
  More |= Static->fuzzyFind(Req, [&](const Symbol ) {
auto DynS = Dyn.find(S.ID);
+   ++StaticCount;
if (DynS == Dyn.end())
  return Callback(S);
+   ++MergedCount;
SeenDynamicSymbols.insert(S.ID);
Callback(mergeSymbol(*DynS, S));
  });
+ SPAN_ATTACH(Tracer, "dynamic", DynamicCount);
+ SPAN_ATTACH(Tracer, "static", StaticCount);
+ SPAN_ATTACH(Tracer, "merged", MergedCount);
  for (const Symbol  : Dyn)
if (!SeenDynamicSymbols.count(S.ID))
  Callback(S);
@@ -60,6 +73,7 @@
   void
   lookup(const LookupRequest ,
  llvm::function_ref Callback) const override {
+trace::Span Tracer("MergedIndex lookup");
 SymbolSlab::Builder B;
 
 Dynamic->lookup(Req, [&](const Symbol ) { B.insert(S); });
@@ -80,6 +94,7 @@
 
   void refs(const RefsRequest ,
 llvm::function_ref Callback) const override {
+trace::Span Tracer("MergedIndex refs");
 // We don't want duplicated refs from the static/dynamic indexes,
 // and we can't reliably duplicate them because offsets may differ slightly.
 // We consider the dynamic index authoritative and report all its refs,
Index: clang-tools-extra/trunk/clangd/index/dex/Dex.cpp
===
--- clang-tools-extra/trunk/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/trunk/clangd/index/dex/Dex.cpp
@@ -12,6 +12,7 @@
 #include "FuzzyMatch.h"
 #include "Logger.h"
 #include "Quality.h"
+#include "Trace.h"
 #include "llvm/ADT/StringSet.h"
 #include 
 #include 
@@ -139,6 +140,8 @@
 llvm::function_ref Callback) const {
   assert(!StringRef(Req.Query).contains("::") &&
  "There must be no :: in query.");
+  // FIXME: attach the query tree to the trace span.
+  trace::Span Tracer("Dex fuzzyFind");

[PATCH] D52611: [clangd] Add more tracing to index queries. NFC

2018-09-27 Thread Eric Liu via Phabricator via cfe-commits 
ioeric updated this revision to Diff 167336.
ioeric marked 2 inline comments as done.
ioeric added a comment.

- address comments.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52611

Files:
  clangd/index/MemIndex.cpp
  clangd/index/Merge.cpp
  clangd/index/dex/Dex.cpp

Index: clangd/index/dex/Dex.cpp
===
--- clangd/index/dex/Dex.cpp
+++ clangd/index/dex/Dex.cpp
@@ -12,6 +12,7 @@
 #include "FuzzyMatch.h"
 #include "Logger.h"
 #include "Quality.h"
+#include "Trace.h"
 #include "llvm/ADT/StringSet.h"
 #include 
 #include 
@@ -139,6 +140,8 @@
 llvm::function_ref Callback) const {
   assert(!StringRef(Req.Query).contains("::") &&
  "There must be no :: in query.");
+  // FIXME: attach the query tree to the trace span.
+  trace::Span Tracer("Dex fuzzyFind");
   FuzzyMatcher Filter(Req.Query);
   bool More = false;
 
@@ -228,6 +231,7 @@
 
 void Dex::lookup(const LookupRequest ,
  llvm::function_ref Callback) const {
+  trace::Span Tracer("Dex lookup");
   for (const auto  : Req.IDs) {
 auto I = LookupTable.find(ID);
 if (I != LookupTable.end())
@@ -237,6 +241,7 @@
 
 void Dex::refs(const RefsRequest ,
llvm::function_ref Callback) const {
+  trace::Span Tracer("Dex refs");
   log("refs is not implemented.");
 }
 
Index: clangd/index/Merge.cpp
===
--- clangd/index/Merge.cpp
+++ clangd/index/Merge.cpp
@@ -9,6 +9,7 @@
 
 #include "Merge.h"
 #include "Logger.h"
+#include "Trace.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/raw_ostream.h"
@@ -38,19 +39,31 @@
  //a) if it's not in the dynamic slab, yield it directly
  //b) if it's in the dynamic slab, merge it and yield the result
  //  3) now yield all the dynamic symbols we haven't processed.
+ trace::Span Tracer("MergedIndex fuzzyFind");
  bool More = false; // We'll be incomplete if either source was.
  SymbolSlab::Builder DynB;
- More |= Dynamic->fuzzyFind(Req, [&](const Symbol ) { DynB.insert(S); });
+ unsigned DynamicCount = 0;
+ unsigned StaticCount = 0;
+ unsigned MergedCount = 0;
+ More |= Dynamic->fuzzyFind(Req, [&](const Symbol ) {
+   ++DynamicCount;
+   DynB.insert(S);
+ });
  SymbolSlab Dyn = std::move(DynB).build();
 
  DenseSet SeenDynamicSymbols;
  More |= Static->fuzzyFind(Req, [&](const Symbol ) {
auto DynS = Dyn.find(S.ID);
+   ++StaticCount;
if (DynS == Dyn.end())
  return Callback(S);
+   ++MergedCount;
SeenDynamicSymbols.insert(S.ID);
Callback(mergeSymbol(*DynS, S));
  });
+ SPAN_ATTACH(Tracer, "dynamic", DynamicCount);
+ SPAN_ATTACH(Tracer, "static", StaticCount);
+ SPAN_ATTACH(Tracer, "merged", MergedCount);
  for (const Symbol  : Dyn)
if (!SeenDynamicSymbols.count(S.ID))
  Callback(S);
@@ -60,6 +73,7 @@
   void
   lookup(const LookupRequest ,
  llvm::function_ref Callback) const override {
+trace::Span Tracer("MergedIndex lookup");
 SymbolSlab::Builder B;
 
 Dynamic->lookup(Req, [&](const Symbol ) { B.insert(S); });
@@ -80,6 +94,7 @@
 
   void refs(const RefsRequest ,
 llvm::function_ref Callback) const override {
+trace::Span Tracer("MergedIndex refs");
 // We don't want duplicated refs from the static/dynamic indexes,
 // and we can't reliably duplicate them because offsets may differ slightly.
 // We consider the dynamic index authoritative and report all its refs,
Index: clangd/index/MemIndex.cpp
===
--- clangd/index/MemIndex.cpp
+++ clangd/index/MemIndex.cpp
@@ -11,6 +11,7 @@
 #include "FuzzyMatch.h"
 #include "Logger.h"
 #include "Quality.h"
+#include "Trace.h"
 
 namespace clang {
 namespace clangd {
@@ -28,6 +29,7 @@
 llvm::function_ref Callback) const {
   assert(!StringRef(Req.Query).contains("::") &&
  "There must be no :: in query.");
+  trace::Span Tracer("MemIndex fuzzyFind");
 
   TopN> Top(
   Req.Limit ? *Req.Limit : std::numeric_limits::max());
@@ -47,13 +49,16 @@
   if (Top.push({*Score * quality(*Sym), Sym}))
 More = true; // An element with smallest score was discarded.
   }
-  for (const auto  : std::move(Top).items())
+  auto Results = std::move(Top).items();
+  SPAN_ATTACH(Tracer, "results", static_cast(Results.size()));
+  for (const auto  : Results)
 Callback(*Item.second);
   return More;
 }
 
 void MemIndex::lookup(const LookupRequest ,
   llvm::function_ref Callback) const {
+  trace::Span Tracer("MemIndex lookup");
   for (const auto  : Req.IDs) {
 auto I = Index.find(ID);
 if (I != Index.end())
@@ -63,6 +68,7 @@
 
 void MemIndex::refs(const RefsRequest ,
 llvm::function_ref Callback) const {
+  trace::Span

[PATCH] D52611: [clangd] Add more tracing to index queries. NFC

2018-09-27 Thread Kirill Bobyrev via Phabricator via cfe-commits 
kbobyrev added inline comments.



Comment at: clangd/index/dex/Dex.cpp:143
  "There must be no :: in query.");
+  trace::Span Tracer("Dex fuzzyFind");
   FuzzyMatcher Filter(Req.Query);

sammccall wrote:
> We should attach the query tree to the span here.
> 
> Right now that's not practical as the dump() representation holds the posting 
> lists rather than their token, but maybe add a FIXME?
Yes, that's an interesting topic. I was thinking about better dump format for 
queries for the `dexp` commands (namely, `find`): right now there's no API to 
pull even the query dump for the external callers, so this is another problem. 
For better format, I was thinking about adding either `LABEL` iterator or 
actually having `StringRef Iterator::label() const;` as most iterators are 
likely to have one. E.g. PostingList's can store the `Token` reference, but 
then we're likely to run into the memory issues so it can be disabled in all 
builds other than `DEBUG`. Other iterators are fine, because they are only 
constructed once per query and never stored afterwards.

And it would be very useful to see the full query structure for both debugging 
purposes and for understanding which queries are slow (and why). I thought 
about measuring distinct queries latency to have better statistics (latency 
histogram & distributions) to understand how to optimize Dex and improve 
performance.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52611



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52611: [clangd] Add more tracing to index queries. NFC

2018-09-27 Thread Sam McCall via Phabricator via cfe-commits 
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clangd/index/Merge.cpp:42
  //  3) now yield all the dynamic symbols we haven't processed.
+ trace::Span Tracer("MergedIndex fuzzyFind");
  bool More = false; // We'll be incomplete if either source was.

might be fun to add static/dynamic/both counts



Comment at: clangd/index/dex/Dex.cpp:143
  "There must be no :: in query.");
+  trace::Span Tracer("Dex fuzzyFind");
   FuzzyMatcher Filter(Req.Query);

We should attach the query tree to the span here.

Right now that's not practical as the dump() representation holds the posting 
lists rather than their token, but maybe add a FIXME?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52611



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52611: [clangd] Add more tracing to index queries. NFC

2018-09-27 Thread Eric Liu via Phabricator via cfe-commits 
ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
ilya-biryukov.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52611

Files:
  clangd/index/MemIndex.cpp
  clangd/index/Merge.cpp
  clangd/index/dex/Dex.cpp

Index: clangd/index/dex/Dex.cpp
===
--- clangd/index/dex/Dex.cpp
+++ clangd/index/dex/Dex.cpp
@@ -12,6 +12,7 @@
 #include "FuzzyMatch.h"
 #include "Logger.h"
 #include "Quality.h"
+#include "Trace.h"
 #include "llvm/ADT/StringSet.h"
 #include 
 #include 
@@ -139,6 +140,7 @@
 llvm::function_ref Callback) const {
   assert(!StringRef(Req.Query).contains("::") &&
  "There must be no :: in query.");
+  trace::Span Tracer("Dex fuzzyFind");
   FuzzyMatcher Filter(Req.Query);
   bool More = false;
 
@@ -228,6 +230,7 @@
 
 void Dex::lookup(const LookupRequest ,
  llvm::function_ref Callback) const {
+  trace::Span Tracer("Dex lookup");
   for (const auto  : Req.IDs) {
 auto I = LookupTable.find(ID);
 if (I != LookupTable.end())
@@ -237,6 +240,7 @@
 
 void Dex::refs(const RefsRequest ,
llvm::function_ref Callback) const {
+  trace::Span Tracer("Dex refs");
   log("refs is not implemented.");
 }
 
Index: clangd/index/Merge.cpp
===
--- clangd/index/Merge.cpp
+++ clangd/index/Merge.cpp
@@ -9,6 +9,7 @@
 
 #include "Merge.h"
 #include "Logger.h"
+#include "Trace.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/raw_ostream.h"
@@ -38,6 +39,7 @@
  //a) if it's not in the dynamic slab, yield it directly
  //b) if it's in the dynamic slab, merge it and yield the result
  //  3) now yield all the dynamic symbols we haven't processed.
+ trace::Span Tracer("MergedIndex fuzzyFind");
  bool More = false; // We'll be incomplete if either source was.
  SymbolSlab::Builder DynB;
  More |= Dynamic->fuzzyFind(Req, [&](const Symbol ) { DynB.insert(S); });
@@ -60,6 +62,7 @@
   void
   lookup(const LookupRequest ,
  llvm::function_ref Callback) const override {
+trace::Span Tracer("MergedIndex lookup");
 SymbolSlab::Builder B;
 
 Dynamic->lookup(Req, [&](const Symbol ) { B.insert(S); });
@@ -80,6 +83,7 @@
 
   void refs(const RefsRequest ,
 llvm::function_ref Callback) const override {
+trace::Span Tracer("MergedIndex refs");
 // We don't want duplicated refs from the static/dynamic indexes,
 // and we can't reliably duplicate them because offsets may differ slightly.
 // We consider the dynamic index authoritative and report all its refs,
Index: clangd/index/MemIndex.cpp
===
--- clangd/index/MemIndex.cpp
+++ clangd/index/MemIndex.cpp
@@ -11,6 +11,7 @@
 #include "FuzzyMatch.h"
 #include "Logger.h"
 #include "Quality.h"
+#include "Trace.h"
 
 namespace clang {
 namespace clangd {
@@ -28,6 +29,7 @@
 llvm::function_ref Callback) const {
   assert(!StringRef(Req.Query).contains("::") &&
  "There must be no :: in query.");
+  trace::Span Tracer("MemIndex fuzzyFind");
 
   TopN> Top(
   Req.Limit ? *Req.Limit : std::numeric_limits::max());
@@ -47,13 +49,16 @@
   if (Top.push({*Score * quality(*Sym), Sym}))
 More = true; // An element with smallest score was discarded.
   }
-  for (const auto  : std::move(Top).items())
+  auto Results = std::move(Top).items();
+  SPAN_ATTACH(Tracer, "results", static_cast(Results.size()));
+  for (const auto  : Results)
 Callback(*Item.second);
   return More;
 }
 
 void MemIndex::lookup(const LookupRequest ,
   llvm::function_ref Callback) const {
+  trace::Span Tracer("MemIndex lookup");
   for (const auto  : Req.IDs) {
 auto I = Index.find(ID);
 if (I != Index.end())
@@ -63,6 +68,7 @@
 
 void MemIndex::refs(const RefsRequest ,
 llvm::function_ref Callback) const {
+  trace::Span Tracer("MemIndex refs");
   for (const auto  : Req.IDs) {
 auto SymRefs = Refs.find(ReqID);
 if (SymRefs == Refs.end())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits