kadircet updated this revision to Diff 261694. kadircet marked an inline comment as done. kadircet added a comment.
- Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79302/new/ https://reviews.llvm.org/D79302 Files: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp clang-tools-extra/clangd/unittests/LSPClient.cpp Index: clang-tools-extra/clangd/unittests/LSPClient.cpp =================================================================== --- clang-tools-extra/clangd/unittests/LSPClient.cpp +++ clang-tools-extra/clangd/unittests/LSPClient.cpp @@ -5,6 +5,7 @@ #include "Protocol.h" #include "TestFS.h" #include "Transport.h" +#include "support/Context.h" #include "support/Threading.h" #include "llvm/Support/Path.h" #include "llvm/Support/raw_ostream.h" @@ -68,7 +69,7 @@ // A null action causes the transport to shut down. void enqueue(std::function<void(MessageHandler &)> Action) { std::lock_guard<std::mutex> Lock(Mu); - Actions.push(std::move(Action)); + Requests.push({Context::current().clone(), std::move(Action)}); CV.notify_all(); } @@ -112,20 +113,28 @@ llvm::Error loop(MessageHandler &H) override { std::unique_lock<std::mutex> Lock(Mu); while (true) { - CV.wait(Lock, [&] { return !Actions.empty(); }); - if (!Actions.front()) // Stop! + CV.wait(Lock, [&] { return !Requests.empty(); }); + if (!Requests.front().Action) // Stop! return llvm::Error::success(); - auto Action = std::move(Actions.front()); - Actions.pop(); + auto Req = std::move(Requests.front()); + Requests.pop(); Lock.unlock(); - Action(H); + { + WithContext Ctx(std::move(Req.Ctx)); + Req.Action(H); + } Lock.lock(); } } + struct Request { + Context Ctx; + std::function<void(Transport::MessageHandler &)> Action; + }; + std::mutex Mu; std::deque<CallResult> CallResults; - std::queue<std::function<void(Transport::MessageHandler &)>> Actions; + std::queue<Request> Requests; std::condition_variable CV; llvm::StringMap<std::vector<llvm::json::Value>> Notifications; }; Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp +++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp @@ -13,8 +13,11 @@ #include "Protocol.h" #include "TestFS.h" #include "refactor/Rename.h" +#include "support/Context.h" #include "support/Logger.h" #include "support/TestTracer.h" +#include "support/Threading.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" #include "llvm/Support/JSON.h" @@ -149,6 +152,21 @@ llvm::ValueIs(testing::ElementsAre( DiagMessage("Use of undeclared identifier 'changed'")))); } + +TEST_F(LSPTest, RecordsLatencies) { + trace::TestTracer Tracer; + auto &Client = start(); + llvm::StringLiteral MethodName = "method_name"; + EXPECT_THAT(Tracer.takeMetric("lsp_latency", MethodName), testing::SizeIs(0)); + Notification CallFinished; + { + WithContextValue Ctx( + llvm::make_scope_exit([&CallFinished] { CallFinished.notify(); })); + llvm::consumeError(Client.call(MethodName, {}).take().takeError()); + } + CallFinished.wait(); + EXPECT_THAT(Tracer.takeMetric("lsp_latency", MethodName), testing::SizeIs(1)); +} } // namespace } // namespace clangd } // namespace clang
Index: clang-tools-extra/clangd/unittests/LSPClient.cpp =================================================================== --- clang-tools-extra/clangd/unittests/LSPClient.cpp +++ clang-tools-extra/clangd/unittests/LSPClient.cpp @@ -5,6 +5,7 @@ #include "Protocol.h" #include "TestFS.h" #include "Transport.h" +#include "support/Context.h" #include "support/Threading.h" #include "llvm/Support/Path.h" #include "llvm/Support/raw_ostream.h" @@ -68,7 +69,7 @@ // A null action causes the transport to shut down. void enqueue(std::function<void(MessageHandler &)> Action) { std::lock_guard<std::mutex> Lock(Mu); - Actions.push(std::move(Action)); + Requests.push({Context::current().clone(), std::move(Action)}); CV.notify_all(); } @@ -112,20 +113,28 @@ llvm::Error loop(MessageHandler &H) override { std::unique_lock<std::mutex> Lock(Mu); while (true) { - CV.wait(Lock, [&] { return !Actions.empty(); }); - if (!Actions.front()) // Stop! + CV.wait(Lock, [&] { return !Requests.empty(); }); + if (!Requests.front().Action) // Stop! return llvm::Error::success(); - auto Action = std::move(Actions.front()); - Actions.pop(); + auto Req = std::move(Requests.front()); + Requests.pop(); Lock.unlock(); - Action(H); + { + WithContext Ctx(std::move(Req.Ctx)); + Req.Action(H); + } Lock.lock(); } } + struct Request { + Context Ctx; + std::function<void(Transport::MessageHandler &)> Action; + }; + std::mutex Mu; std::deque<CallResult> CallResults; - std::queue<std::function<void(Transport::MessageHandler &)>> Actions; + std::queue<Request> Requests; std::condition_variable CV; llvm::StringMap<std::vector<llvm::json::Value>> Notifications; }; Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp +++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp @@ -13,8 +13,11 @@ #include "Protocol.h" #include "TestFS.h" #include "refactor/Rename.h" +#include "support/Context.h" #include "support/Logger.h" #include "support/TestTracer.h" +#include "support/Threading.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" #include "llvm/Support/JSON.h" @@ -149,6 +152,21 @@ llvm::ValueIs(testing::ElementsAre( DiagMessage("Use of undeclared identifier 'changed'")))); } + +TEST_F(LSPTest, RecordsLatencies) { + trace::TestTracer Tracer; + auto &Client = start(); + llvm::StringLiteral MethodName = "method_name"; + EXPECT_THAT(Tracer.takeMetric("lsp_latency", MethodName), testing::SizeIs(0)); + Notification CallFinished; + { + WithContextValue Ctx( + llvm::make_scope_exit([&CallFinished] { CallFinished.notify(); })); + llvm::consumeError(Client.call(MethodName, {}).take().takeError()); + } + CallFinished.wait(); + EXPECT_THAT(Tracer.takeMetric("lsp_latency", MethodName), testing::SizeIs(1)); +} } // namespace } // namespace clangd } // namespace clang
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits