From 42fc527fec0828b105cba264eaf4cf73881a6cf8 Mon Sep 17 00:00:00 2001 From: David Schulz Date: Mon, 11 Apr 2022 07:28:29 +0200 Subject: [PATCH] LSP: Simplify content parsing JsonRpcMessageHandler::toJsonObject can be expensive for huge json objects like the clangd ast responses for big c++ files. Avoid calling this function in Request::responseHandler. We already generate a JsonRpcMessage in JsonRpcMessageHandler::parseContent to get the id of the response so pass this around. While at it also pass around references instead of pointers to simplify the memory handling of generated messages. Change-Id: I9a3c7e85428fc064d1ea1197d897739725265192 Reviewed-by: Christian Stenger --- src/libs/languageserverprotocol/icontent.h | 6 +- .../jsonrpcmessages.cpp | 26 ++-- .../languageserverprotocol/jsonrpcmessages.h | 28 +--- src/plugins/languageclient/client.cpp | 137 +++++++++--------- src/plugins/languageclient/client.h | 9 +- .../languageclient/languageclientmanager.cpp | 11 -- 6 files changed, 94 insertions(+), 123 deletions(-) diff --git a/src/libs/languageserverprotocol/icontent.h b/src/libs/languageserverprotocol/icontent.h index 9173bda66ec..4a19955e148 100644 --- a/src/libs/languageserverprotocol/icontent.h +++ b/src/libs/languageserverprotocol/icontent.h @@ -94,12 +94,12 @@ private: struct ResponseHandler { MessageId id; - using Callback = std::function; + using Callback = std::function; Callback callback; }; -using ResponseHandlers = std::function; -using MethodHandler = std::function; +using ResponseHandlers = std::function; +using MethodHandler = std::function; template inline QDebug operator<<(QDebug stream, const LanguageServerProtocol::MessageId &id) diff --git a/src/libs/languageserverprotocol/jsonrpcmessages.cpp b/src/libs/languageserverprotocol/jsonrpcmessages.cpp index abd19f4b68f..517ffd5f8fb 100644 --- a/src/libs/languageserverprotocol/jsonrpcmessages.cpp +++ b/src/libs/languageserverprotocol/jsonrpcmessages.cpp @@ -40,8 +40,6 @@ Q_LOGGING_CATEGORY(timingLog, "qtc.languageserverprotocol.timing", QtWarningMsg) constexpr const char CancelRequest::methodName[]; -QHash JsonRpcMessageHandler::m_messageProvider; - QByteArray JsonRpcMessage::toRawData() const { return QJsonDocument(m_jsonObject).toJson(QJsonDocument::Compact); @@ -57,6 +55,11 @@ bool JsonRpcMessage::isValid(QString * /*errorMessage*/) const return m_jsonObject[jsonRpcVersionKey] == "2.0"; } +const QJsonObject &JsonRpcMessage::toJsonObject() const +{ + return m_jsonObject; +} + JsonRpcMessage::JsonRpcMessage() { // The language server protocol always uses “2.0” as the jsonrpc version @@ -76,12 +79,6 @@ QByteArray JsonRpcMessageHandler::jsonRpcMimeType() return "application/vscode-jsonrpc"; } -void JsonRpcMessageHandler::registerMessageProvider( - const QString &method, JsonRpcMessageHandler::MessageProvider provider) -{ - m_messageProvider.insert(method, provider); -} - void JsonRpcMessageHandler::parseContent(const QByteArray &content, QTextCodec *codec, QString &parseError, @@ -94,14 +91,11 @@ void JsonRpcMessageHandler::parseContent(const QByteArray &content, const MessageId id(jsonObject.value(idKey)); const QString &method = jsonObject.value(methodKey).toString(); - if (!method.isEmpty()) { - if (auto provider = m_messageProvider[method]) { - methodHandler(method, id, provider(jsonObject)); - return; - } - } - - responseHandlers(id, content, codec); + const JsonRpcMessage jsonContent(jsonObject); + if (method.isEmpty()) + responseHandlers(id, jsonContent); + else + methodHandler(method, id, jsonContent); } constexpr int utf8mib = 106; diff --git a/src/libs/languageserverprotocol/jsonrpcmessages.h b/src/libs/languageserverprotocol/jsonrpcmessages.h index ad11624a22a..7b24bc9b901 100644 --- a/src/libs/languageserverprotocol/jsonrpcmessages.h +++ b/src/libs/languageserverprotocol/jsonrpcmessages.h @@ -54,6 +54,7 @@ public: QByteArray mimeType() const final; bool isValid(QString *errorMessage) const override; + const QJsonObject &toJsonObject() const; protected: QJsonObject m_jsonObject; @@ -66,23 +67,11 @@ class LANGUAGESERVERPROTOCOL_EXPORT JsonRpcMessageHandler Q_DECLARE_TR_FUNCTIONS(JsonRpcMessageHandler) public: - using MessageProvider = std::function; - static void registerMessageProvider(const QString &method, MessageProvider provider); - template - static void registerMessageProvider() - { - registerMessageProvider(T::methodName, [](const QJsonObject &object){ - return new T(object); - }); - } static QByteArray jsonRpcMimeType(); static void parseContent(const QByteArray &content, QTextCodec *codec, QString &errorMessage, const ResponseHandlers &responseHandlers, const MethodHandler &methodHandler); static QJsonObject toJsonObject(const QByteArray &content, QTextCodec *codec, QString &parseError); - -private: - static QHash m_messageProvider; }; template @@ -322,21 +311,12 @@ public: QElapsedTimer timer; timer.start(); auto callback = [callback = m_callBack, method = this->method(), t = std::move(timer)] - (const QByteArray &content, QTextCodec *codec) { + (const IContent &content) { if (!callback) return; logElapsedTime(method, t); - QString parseError; - const QJsonObject &object = JsonRpcMessageHandler::toJsonObject(content, - codec, - parseError); - Response response(object); - if (object.isEmpty()) { - ResponseError error; - error.setMessage(parseError); - response.setError(error); - } - callback(Response(object)); + + callback(Response(static_cast(content).toJsonObject())); }; return Utils::make_optional(ResponseHandler{id(), callback}); } diff --git a/src/plugins/languageclient/client.cpp b/src/plugins/languageclient/client.cpp index becb4c3ab47..e6b57e70376 100644 --- a/src/plugins/languageclient/client.cpp +++ b/src/plugins/languageclient/client.cpp @@ -1205,10 +1205,10 @@ void Client::handleMessage(const BaseMessage &message) if (auto handler = m_contentHandler[message.mimeType]) { QString parseError; handler(message.content, message.codec, parseError, - [this](const MessageId &id, const QByteArray &content, QTextCodec *codec){ - this->handleResponse(id, content, codec); + [this](const MessageId &id, const IContent &content){ + this->handleResponse(id, content); }, - [this](const QString &method, const MessageId &id, const IContent *content){ + [this](const QString &method, const MessageId &id, const IContent &content){ this->handleMethod(method, id, content); }); if (!parseError.isEmpty()) @@ -1346,10 +1346,10 @@ void Client::sendPostponedDocumentUpdates(Schedule semanticTokensSchedule) } } -void Client::handleResponse(const MessageId &id, const QByteArray &content, QTextCodec *codec) +void Client::handleResponse(const MessageId &id, const IContent &content) { if (auto handler = m_responseHandlers[id]) - handler(content, codec); + handler(content); } template @@ -1361,118 +1361,137 @@ static ResponseError createInvalidParamsError(const QString &message) return error; } -void Client::handleMethod(const QString &method, const MessageId &id, const IContent *content) +template +static T asJsonContent(const IContent &content) { + return T(static_cast(content).toJsonObject()); +} + +void Client::handleMethod(const QString &method, const MessageId &id, const IContent &content) { auto invalidParamsErrorMessage = [&](const JsonObject ¶ms) { return tr("Invalid parameter in \"%1\":\n%2") .arg(method, QString::fromUtf8(QJsonDocument(params).toJson(QJsonDocument::Indented))); }; - auto createDefaultResponse = [&]() -> IContent * { - Response *response = nullptr; - if (id.isValid()) { - response = new Response(id); - response->setResult(nullptr); - } + auto createDefaultResponse = [&]() { + Response response; + if (QTC_GUARD(id.isValid())) + response.setId(id); + response.setResult(nullptr); return response; }; const bool isRequest = id.isValid(); - IContent *response = nullptr; + + bool responseSend = false; + auto sendResponse = + [&](const IContent &response) { + responseSend = true; + if (reachable()) { + sendContent(response); + } else { + qCDebug(LOGLSPCLIENT) + << QString("Dropped response to request %1 id %2 for unreachable server %3") + .arg(method, id.toString(), name()); + } + }; if (method == PublishDiagnosticsNotification::methodName) { - auto params = dynamic_cast(content)->params().value_or(PublishDiagnosticsParams()); + auto params = asJsonContent(content).params().value_or( + PublishDiagnosticsParams()); if (params.isValid()) handleDiagnostics(params); else log(invalidParamsErrorMessage(params)); } else if (method == LogMessageNotification::methodName) { - auto params = dynamic_cast(content)->params().value_or(LogMessageParams()); + auto params = asJsonContent(content).params().value_or( + LogMessageParams()); if (params.isValid()) log(params); else log(invalidParamsErrorMessage(params)); } else if (method == ShowMessageNotification::methodName) { - auto params = dynamic_cast(content)->params().value_or(ShowMessageParams()); + auto params = asJsonContent(content).params().value_or( + ShowMessageParams()); if (params.isValid()) log(params); else log(invalidParamsErrorMessage(params)); } else if (method == ShowMessageRequest::methodName) { - auto request = dynamic_cast(content); - auto showMessageResponse = new ShowMessageRequest::Response(id); - auto params = request->params().value_or(ShowMessageRequestParams()); + auto request = asJsonContent(content); + ShowMessageRequest::Response response(id); + auto params = request.params().value_or(ShowMessageRequestParams()); if (params.isValid()) { - showMessageResponse->setResult(showMessageBox(params)); + response.setResult(showMessageBox(params)); } else { const QString errorMessage = invalidParamsErrorMessage(params); log(errorMessage); - showMessageResponse->setError(createInvalidParamsError(errorMessage)); + response.setError(createInvalidParamsError(errorMessage)); } - response = showMessageResponse; + sendResponse(response); } else if (method == RegisterCapabilityRequest::methodName) { - auto params = dynamic_cast(content)->params().value_or( + auto params = asJsonContent(content).params().value_or( RegistrationParams()); if (params.isValid()) { registerCapabilities(params.registrations()); - response = createDefaultResponse(); + sendResponse(createDefaultResponse()); } else { const QString errorMessage = invalidParamsErrorMessage(params); log(invalidParamsErrorMessage(params)); - auto registerResponse = new RegisterCapabilityRequest::Response(id); - registerResponse->setError(createInvalidParamsError(errorMessage)); - response = registerResponse; + RegisterCapabilityRequest::Response response(id); + response.setError(createInvalidParamsError(errorMessage)); + sendResponse(response); } } else if (method == UnregisterCapabilityRequest::methodName) { - auto params = dynamic_cast(content)->params().value_or( + auto params = asJsonContent(content).params().value_or( UnregistrationParams()); if (params.isValid()) { unregisterCapabilities(params.unregistrations()); - response = createDefaultResponse(); + sendResponse(createDefaultResponse()); } else { const QString errorMessage = invalidParamsErrorMessage(params); log(invalidParamsErrorMessage(params)); - auto registerResponse = new UnregisterCapabilityRequest::Response(id); - registerResponse->setError(createInvalidParamsError(errorMessage)); - response = registerResponse; + UnregisterCapabilityRequest::Response response(id); + response.setError(createInvalidParamsError(errorMessage)); + sendResponse(response); } } else if (method == ApplyWorkspaceEditRequest::methodName) { - auto editResponse = new ApplyWorkspaceEditRequest::Response(id); - auto params = dynamic_cast(content)->params().value_or( + ApplyWorkspaceEditRequest::Response response(id); + auto params = asJsonContent(content).params().value_or( ApplyWorkspaceEditParams()); if (params.isValid()) { ApplyWorkspaceEditResult result; result.setApplied(applyWorkspaceEdit(this, params.edit())); - editResponse->setResult(result); + response.setResult(result); } else { const QString errorMessage = invalidParamsErrorMessage(params); log(errorMessage); - editResponse->setError(createInvalidParamsError(errorMessage)); + response.setError(createInvalidParamsError(errorMessage)); } - response = editResponse; + sendResponse(response); } else if (method == WorkSpaceFolderRequest::methodName) { - auto workSpaceFolderResponse = new WorkSpaceFolderRequest::Response(id); + WorkSpaceFolderRequest::Response response(id); const QList projects = ProjectExplorer::SessionManager::projects(); - WorkSpaceFolderResult result; if (projects.isEmpty()) { - result = nullptr; + response.setResult(nullptr); } else { - result = Utils::transform(projects, [](ProjectExplorer::Project *project) { - return WorkSpaceFolder(DocumentUri::fromFilePath(project->projectDirectory()), - project->displayName()); - }); + response.setResult(Utils::transform( + projects, + [](ProjectExplorer::Project *project) { + return WorkSpaceFolder(DocumentUri::fromFilePath(project->projectDirectory()), + project->displayName()); + })); } - workSpaceFolderResponse->setResult(result); - response = workSpaceFolderResponse; + sendResponse(response); } else if (method == WorkDoneProgressCreateRequest::methodName) { - response = createDefaultResponse(); + sendResponse(createDefaultResponse()); } else if (method == SemanticTokensRefreshRequest::methodName) { m_tokenSupport.refresh(); - response = createDefaultResponse(); + sendResponse(createDefaultResponse()); } else if (method == ProgressNotification::methodName) { if (Utils::optional params - = dynamic_cast(content)->params()) { + = asJsonContent(content).params()) { if (!params->isValid()) log(invalidParamsErrorMessage(*params)); m_progressManager.handleProgress(*params); @@ -1480,27 +1499,15 @@ void Client::handleMethod(const QString &method, const MessageId &id, const ICon emit workDone(params->token()); } } else if (isRequest) { - auto methodNotFoundResponse = new Response(id); + Response response(id); ResponseError error; error.setCode(ResponseError::MethodNotFound); - methodNotFoundResponse->setError(error); - response = methodNotFoundResponse; + response.setError(error); + sendResponse(response); } // we got a request and handled it somewhere above but we missed to generate a response for it - QTC_ASSERT(!isRequest || response, response = createDefaultResponse()); - - if (response) { - if (reachable()) { - sendContent(*response); - } else { - qCDebug(LOGLSPCLIENT) - << QString("Dropped response to request %1 id %2 for unreachable server %3") - .arg(method, id.toString(), name()); - } - delete response; - } - delete content; + QTC_ASSERT(!isRequest || responseSend, sendResponse(createDefaultResponse())); } void Client::handleDiagnostics(const PublishDiagnosticsParams ¶ms) diff --git a/src/plugins/languageclient/client.h b/src/plugins/languageclient/client.h index 4fb656c9a13..1db0b9d7478 100644 --- a/src/plugins/languageclient/client.h +++ b/src/plugins/languageclient/client.h @@ -229,10 +229,11 @@ protected: private: void sendMessage(const LanguageServerProtocol::BaseMessage &message); - void handleResponse(const LanguageServerProtocol::MessageId &id, const QByteArray &content, - QTextCodec *codec); - void handleMethod(const QString &method, const LanguageServerProtocol::MessageId &id, - const LanguageServerProtocol::IContent *content); + void handleResponse(const LanguageServerProtocol::MessageId &id, + const LanguageServerProtocol::IContent &content); + void handleMethod(const QString &method, + const LanguageServerProtocol::MessageId &id, + const LanguageServerProtocol::IContent &content); void initializeCallback(const LanguageServerProtocol::InitializeRequest::Response &initResponse); void shutDownCallback(const LanguageServerProtocol::ShutdownRequest::Response &shutdownResponse); diff --git a/src/plugins/languageclient/languageclientmanager.cpp b/src/plugins/languageclient/languageclientmanager.cpp index 48bb3901fe1..092b95f3717 100644 --- a/src/plugins/languageclient/languageclientmanager.cpp +++ b/src/plugins/languageclient/languageclientmanager.cpp @@ -61,17 +61,6 @@ LanguageClientManager::LanguageClientManager(QObject *parent) { using namespace Core; using namespace ProjectExplorer; - JsonRpcMessageHandler::registerMessageProvider(); - JsonRpcMessageHandler::registerMessageProvider(); - JsonRpcMessageHandler::registerMessageProvider(); - JsonRpcMessageHandler::registerMessageProvider(); - JsonRpcMessageHandler::registerMessageProvider(); - JsonRpcMessageHandler::registerMessageProvider(); - JsonRpcMessageHandler::registerMessageProvider(); - JsonRpcMessageHandler::registerMessageProvider(); - JsonRpcMessageHandler::registerMessageProvider(); - JsonRpcMessageHandler::registerMessageProvider(); - JsonRpcMessageHandler::registerMessageProvider(); connect(EditorManager::instance(), &EditorManager::editorOpened, this, &LanguageClientManager::editorOpened); connect(EditorManager::instance(), &EditorManager::documentOpened,