[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