This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Add more tracing to index queries. NFC
ClosedPublic

Authored by ioeric on Sep 27 2018, 7:10 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

ioeric created this revision.Sep 27 2018, 7:10 AM
sammccall accepted this revision.Sep 27 2018, 7:19 AM
sammccall added inline comments.
clangd/index/Merge.cpp
42 ↗(On Diff #167318)

might be fun to add static/dynamic/both counts

clangd/index/dex/Dex.cpp
143 ↗(On Diff #167318)

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?

This revision is now accepted and ready to land.Sep 27 2018, 7:19 AM
kbobyrev added inline comments.
clangd/index/dex/Dex.cpp
143 ↗(On Diff #167318)

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.

ioeric updated this revision to Diff 167336.Sep 27 2018, 8:59 AM
ioeric marked 2 inline comments as done.
  • address comments.
This revision was automatically updated to reflect the committed changes.