[PATCH] D47935: [clangd] Boost completion score according to file proximity.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Can you benchmark this? I'm nervous about the URI stuff in the hot path.
Timing CodeCompleteFlow::measureResults before/after with index enabled seems like a reasonable test.
(But you might want to make this apply to sema first too for realistic numbers?)



================
Comment at: clangd/Quality.cpp:203
+  StringRef SymURI = IndexResult.CanonicalDeclaration.FileURI;
+  if (!ProximityPath.empty() && !SymURI.empty()) {
+    // Only calculate proximity score for two URIs with the same scheme so that
----------------
how do we know proximitypath is set at this point?
Better to copy the symbol URL path I think :-(


================
Comment at: clangd/Quality.cpp:208
+    if (auto U = URI::create(ProximityPath, SymURI.split(':').first)) {
+      ProximityScore += uriProximity(SymURI, U->toString());
+    } else {
----------------
Why U->toString() rather than ->body()?


================
Comment at: clangd/Quality.cpp:215
 
 void SymbolRelevanceSignals::merge(const CodeCompletionResult &SemaCCResult) {
   if (SemaCCResult.Availability == CXAvailability_NotAvailable ||
----------------
proximity path needs to be set here too


================
Comment at: clangd/Quality.h:74
+  // If set, this is used to compute proximity from symbol's declaring file.
+  llvm::StringRef ProximityPath;
   /// Proximity between best declaration and the query. [0-1], 1 is closest.
----------------
It seems OK to have ProximityPath or ProximityScore, but we shouldn't have both: drop proximityscore and calculate it during evaluate()?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47935