[clang-tools-extra] [clangd] Add container field to remote index Refs grpc method (PR #71605)

via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 28 18:19:37 PST 2024


https://github.com/tdupes updated https://github.com/llvm/llvm-project/pull/71605

>From 4282c39fd0b40c26c68f57257c150b984264f50b Mon Sep 17 00:00:00 2001
From: dup <dup at fb.com>
Date: Fri, 3 Nov 2023 09:03:24 -0700
Subject: [PATCH 1/2] Add container field to remote index Refs grpc method

---
 clang-tools-extra/clangd/index/YAMLSerialization.cpp          | 1 +
 clang-tools-extra/clangd/index/remote/Index.proto             | 1 +
 .../clangd/index/remote/marshalling/Marshalling.cpp           | 4 ++++
 .../clangd/unittests/remote/MarshallingTests.cpp              | 1 +
 4 files changed, 7 insertions(+)

diff --git a/clang-tools-extra/clangd/index/YAMLSerialization.cpp b/clang-tools-extra/clangd/index/YAMLSerialization.cpp
index 214a847b5eddb3..495d8a2ff487aa 100644
--- a/clang-tools-extra/clangd/index/YAMLSerialization.cpp
+++ b/clang-tools-extra/clangd/index/YAMLSerialization.cpp
@@ -319,6 +319,7 @@ template <> struct MappingTraits<Ref> {
     MappingNormalization<NormalizedRefKind, RefKind> NKind(IO, R.Kind);
     IO.mapRequired("Kind", NKind->Kind);
     IO.mapRequired("Location", R.Location);
+    IO.mapOptional("Container", R.Container);
   }
 };
 
diff --git a/clang-tools-extra/clangd/index/remote/Index.proto b/clang-tools-extra/clangd/index/remote/Index.proto
index 3072299d8f345f..33bf095d88598f 100644
--- a/clang-tools-extra/clangd/index/remote/Index.proto
+++ b/clang-tools-extra/clangd/index/remote/Index.proto
@@ -81,6 +81,7 @@ message Symbol {
 message Ref {
   optional SymbolLocation location = 1;
   optional uint32 kind = 2;
+  optional string container = 3;
 }
 
 message SymbolInfo {
diff --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
index 7e31ada18a6579..e9f0972ba91c3f 100644
--- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -189,6 +189,9 @@ llvm::Expected<clangd::Ref> Marshaller::fromProtobuf(const Ref &Message) {
     return Location.takeError();
   Result.Location = *Location;
   Result.Kind = static_cast<RefKind>(Message.kind());
+  auto ContainerID = SymbolID::fromStr(Message.container());
+  if (ContainerID)
+    Result.Container = *ContainerID;
   return Result;
 }
 
@@ -296,6 +299,7 @@ llvm::Expected<Ref> Marshaller::toProtobuf(const clangd::Ref &From) {
   if (!Location)
     return Location.takeError();
   *Result.mutable_location() = *Location;
+  Result.set_container(From.Container.str());
   return Result;
 }
 
diff --git a/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp b/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
index a2b31698a059a9..85e79eae331bc1 100644
--- a/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
@@ -223,6 +223,7 @@ TEST(RemoteMarshallingTest, RefSerialization) {
   Location.FileURI = testPathURI(
       "llvm-project/llvm/clang-tools-extra/clangd/Protocol.h", Strings);
   Ref.Location = Location;
+  Ref.Container = llvm::cantFail(SymbolID::fromStr("0000000000000001"));
 
   Marshaller ProtobufMarshaller(testPath("llvm-project/"),
                                 testPath("llvm-project/"));

>From 81271a1a536ca950eba328e5e84571e20a37d46e Mon Sep 17 00:00:00 2001
From: dup <dup at fb.com>
Date: Wed, 28 Feb 2024 18:19:19 -0800
Subject: [PATCH 2/2] [clangd] Include Json Request ID in trace events

---
 clang-tools-extra/clangd/ClangdLSPServer.cpp       | 14 ++++++++++++--
 clang-tools-extra/clangd/ClangdServer.cpp          |  7 ++++++-
 clang-tools-extra/clangd/support/Trace.cpp         | 12 ++++++++++++
 clang-tools-extra/clangd/support/Trace.h           |  2 ++
 .../clangd/unittests/support/TraceTests.cpp        |  3 +++
 5 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index f29dadde2b86d5..562d33a362cc90 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -229,11 +229,12 @@ class ClangdLSPServer::MessageHandler : public Transport::MessageHandler {
 
   bool onCall(llvm::StringRef Method, llvm::json::Value Params,
               llvm::json::Value ID) override {
-    WithContext HandlerContext(handlerContext());
+    WithContext HandlerContext(handlerContext(llvm::to_string(ID)));
     // Calls can be canceled by the client. Add cancellation context.
     WithContext WithCancel(cancelableRequestContext(ID));
     trace::Span Tracer(Method, LSPLatency);
     SPAN_ATTACH(Tracer, "Params", Params);
+    SPAN_ATTACH(Tracer, "Rid", ID);
     ReplyOnce Reply(ID, Method, &Server, Tracer.Args);
     log("<-- {0}({1})", Method, ID);
     auto Handler = Server.Handlers.MethodHandlers.find(Method);
@@ -252,7 +253,7 @@ class ClangdLSPServer::MessageHandler : public Transport::MessageHandler {
 
   bool onReply(llvm::json::Value ID,
                llvm::Expected<llvm::json::Value> Result) override {
-    WithContext HandlerContext(handlerContext());
+    WithContext HandlerContext(handlerContext(llvm::to_string(ID)));
 
     Callback<llvm::json::Value> ReplyHandler = nullptr;
     if (auto IntID = ID.getAsInteger()) {
@@ -415,6 +416,15 @@ class ClangdLSPServer::MessageHandler : public Transport::MessageHandler {
         Server.Opts.Encoding.value_or(OffsetEncoding::UTF16));
   }
 
+  Context handlerContext(const std::string& ID) const {
+    return Context::current().derive(
+        kCurrentOffsetEncoding,
+        Server.Opts.Encoding.value_or(OffsetEncoding::UTF16)).derive(
+          trace::EventTracer::kRequestId,
+          ID
+        );
+  }
+
   // We run cancelable requests in a context that does two things:
   //  - allows cancellation using RequestCancelers[ID]
   //  - cleans up the entry in RequestCancelers when it's no longer needed
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index 3f9fd012819428..48be7576f87955 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -781,10 +781,15 @@ void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID,
 
 void ClangdServer::locateSymbolAt(PathRef File, Position Pos,
                                   Callback<std::vector<LocatedSymbol>> CB) {
-  auto Action = [Pos, CB = std::move(CB),
+  // Copy over the request ID from the current thread to the worker thread action
+  const auto rid = Context::current().get(trace::EventTracer::kRequestId);
+  auto Action = [Pos, CB = std::move(CB), rid,
                  this](llvm::Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
       return CB(InpAST.takeError());
+    if (rid)
+      Context::current().derive(trace::EventTracer::kRequestId,
+          *rid);
     CB(clangd::locateSymbolAt(InpAST->AST, Pos, Index));
   };
 
diff --git a/clang-tools-extra/clangd/support/Trace.cpp b/clang-tools-extra/clangd/support/Trace.cpp
index 419c2eee99ec84..78c4dda6588d31 100644
--- a/clang-tools-extra/clangd/support/Trace.cpp
+++ b/clang-tools-extra/clangd/support/Trace.cpp
@@ -24,6 +24,8 @@ namespace clang {
 namespace clangd {
 namespace trace {
 
+Key<std::string> EventTracer::kRequestId;
+
 namespace {
 // The current implementation is naive: each thread writes to Out guarded by Mu.
 // Perhaps we should replace this by something that disturbs performance less.
@@ -84,6 +86,12 @@ class JSONTracer : public EventTracer {
     rawEvent(Phase, Contents);
   }
 
+  /// Called whenever a metrics records a measurement.
+  void record(const Metric &Metric, double Value,
+                      llvm::StringRef Label) override {
+      instant("Metric", llvm::json::Object{{"name", Metric.Name}, {"value", Value}, {"label", Label}});
+  }
+
 private:
   class JSONSpan {
   public:
@@ -159,6 +167,10 @@ class JSONTracer : public EventTracer {
     Out.object([&] {
       Out.attribute("pid", 0);
       Out.attribute("ph", Phase);
+      const auto *id = Context::current().get(kRequestId);
+      if (id) {
+        Out.attribute("rid", *id);
+      }
       for (const auto &KV : Event)
         Out.attribute(KV.first, KV.second);
     });
diff --git a/clang-tools-extra/clangd/support/Trace.h b/clang-tools-extra/clangd/support/Trace.h
index 1bfc75b874d8a9..ae00ea322fb39a 100644
--- a/clang-tools-extra/clangd/support/Trace.h
+++ b/clang-tools-extra/clangd/support/Trace.h
@@ -99,6 +99,8 @@ class EventTracer {
   /// Called whenever a metrics records a measurement.
   virtual void record(const Metric &Metric, double Value,
                       llvm::StringRef Label) {}
+  /// A key to store json request id in the context used in tracers
+  static Key<std::string> kRequestId;
 };
 
 /// Sets up a global EventTracer that consumes events produced by Span and
diff --git a/clang-tools-extra/clangd/unittests/support/TraceTests.cpp b/clang-tools-extra/clangd/unittests/support/TraceTests.cpp
index b73c7108ddde6f..0277ab539f64a0 100644
--- a/clang-tools-extra/clangd/unittests/support/TraceTests.cpp
+++ b/clang-tools-extra/clangd/unittests/support/TraceTests.cpp
@@ -124,6 +124,9 @@ TEST(TraceTest, SmokeTest) {
   EXPECT_TRUE(verifyObject(*Event, {{"ph", "i"}, {"name", "Log"}}));
   ASSERT_NE(++Event, Events->end()) << "Expected span end";
   EXPECT_TRUE(verifyObject(*Event, {{"ph", "X"}, {"name", "A"}}));
+  ASSERT_NE(++Event, Events->end());
+  // New Metric instant events included in trace.
+  EXPECT_TRUE(verifyObject(*Event, {{"ph", "i"}, {"name", "Metric"}}));
   ASSERT_EQ(++Event, Events->end());
   ASSERT_EQ(++Prop, Root->end());
 }



More information about the cfe-commits mailing list