# HG changeset patch # User Sebastien Jodogne # Date 1620118662 -7200 # Node ID 9804d6490872f3c37489eebe1adb01733455cf3d # Parent e915102093de1394c60a5ac182ae562c770d8d68 Reduced memory consumption of HTTP/REST plugins calls on POST/PUT if chunked transfer is disabled diff -r e915102093de -r 9804d6490872 NEWS --- a/NEWS Fri Apr 30 10:09:50 2021 +0200 +++ b/NEWS Tue May 04 10:57:42 2021 +0200 @@ -8,6 +8,8 @@ * Fix regression in the handling of "DicomCheckModalityHost" configuration option introduced by changeset 4182 in Orthanc 1.7.4 * New CMake option: "ORTHANC_LUA_VERSION" to use a specific version of system-wide Lua +* Reduced memory consumption of "OrthancPluginHttpClient()", "OrthancPluginHttpClient2()" and + "OrthancPluginCallPeerApi()" on POST/PUT if chunked transfer is disabled Version 1.9.2 (2021-04-22) diff -r e915102093de -r 9804d6490872 OrthancFramework/Sources/HttpClient.cpp --- a/OrthancFramework/Sources/HttpClient.cpp Fri Apr 30 10:09:50 2021 +0200 +++ b/OrthancFramework/Sources/HttpClient.cpp Tue May 04 10:57:42 2021 +0200 @@ -622,6 +622,10 @@ timeout_ = GlobalParameters::GetInstance().GetDefaultTimeout(); GlobalParameters::GetInstance().GetDefaultProxy(proxy_); GlobalParameters::GetInstance().GetSslConfiguration(verifyPeers_, caCertificates_); + + hasExternalBody_ = false; + externalBodyData_ = NULL; + externalBodySize_ = 0; } @@ -718,20 +722,28 @@ } - void HttpClient::SetBody(const std::string& data) + void HttpClient::AssignBody(const std::string& data) { body_ = data; pimpl_->requestBody_.Clear(); + hasExternalBody_ = false; } - std::string &HttpClient::GetBody() + + void HttpClient::AssignBody(const void* data, + size_t size) { - return body_; - } - - const std::string &HttpClient::GetBody() const - { - return body_; + if (size != 0 && + data == NULL) + { + throw OrthancException(ErrorCode_NullPointer); + } + else + { + body_.assign(reinterpret_cast(data), size); + pimpl_->requestBody_.Clear(); + hasExternalBody_ = false; + } } @@ -739,13 +751,34 @@ { body_.clear(); pimpl_->requestBody_.SetBody(body); + hasExternalBody_ = false; } + void HttpClient::SetExternalBody(const void* data, + size_t size) + { + if (size != 0 && + data == NULL) + { + throw OrthancException(ErrorCode_NullPointer); + } + else + { + body_.clear(); + pimpl_->requestBody_.Clear(); + hasExternalBody_ = true; + externalBodyData_ = data; + externalBodySize_ = size; + } + } + + void HttpClient::ClearBody() { body_.clear(); pimpl_->requestBody_.Clear(); + hasExternalBody_ = false; } @@ -976,7 +1009,12 @@ pimpl_->defaultPostHeaders_.Assign(pimpl_->curl_); } - if (body_.size() > 0) + if (hasExternalBody_) + { + CheckCode(curl_easy_setopt(pimpl_->curl_, CURLOPT_POSTFIELDS, externalBodyData_)); + CheckCode(curl_easy_setopt(pimpl_->curl_, CURLOPT_POSTFIELDSIZE, externalBodySize_)); + } + else if (body_.size() > 0) { CheckCode(curl_easy_setopt(pimpl_->curl_, CURLOPT_POSTFIELDS, body_.c_str())); CheckCode(curl_easy_setopt(pimpl_->curl_, CURLOPT_POSTFIELDSIZE, body_.size())); diff -r e915102093de -r 9804d6490872 OrthancFramework/Sources/HttpClient.h --- a/OrthancFramework/Sources/HttpClient.h Fri Apr 30 10:09:50 2021 +0200 +++ b/OrthancFramework/Sources/HttpClient.h Tue May 04 10:57:42 2021 +0200 @@ -106,6 +106,11 @@ bool headersToLowerCase_; bool redirectionFollowed_; + // New in Orthanc 1.9.3 to avoid memcpy() + bool hasExternalBody_; + const void* externalBodyData_; + size_t externalBodySize_; + void Setup(); void operator= (const HttpClient&); // Assignment forbidden @@ -141,14 +146,23 @@ long GetTimeout() const; - void SetBody(const std::string& data); + void AssignBody(const std::string& data); - std::string& GetBody(); - - const std::string& GetBody() const; + void AssignBody(const void* data, + size_t size); void SetBody(IRequestBody& body); + // New in Orthanc 1.9.3: The "data" buffer must have a lifetime + // that is longer than the HttpClient object + void SetExternalBody(const void* data, + size_t size); + + void SetExternalBody(const std::string& data) + { + SetExternalBody(data.empty() ? NULL : data.c_str(), data.size()); + } + void ClearBody(); void SetVerbose(bool isVerbose); diff -r e915102093de -r 9804d6490872 OrthancFramework/Sources/Lua/LuaContext.cpp --- a/OrthancFramework/Sources/Lua/LuaContext.cpp Fri Apr 30 10:09:50 2021 +0200 +++ b/OrthancFramework/Sources/Lua/LuaContext.cpp Tue May 04 10:57:42 2021 +0200 @@ -239,7 +239,7 @@ const char* url = lua_tostring(state, 1); that.httpClient_.SetMethod(HttpMethod_Get); that.httpClient_.SetUrl(url); - that.httpClient_.GetBody().clear(); + that.httpClient_.ClearBody(); that.SetHttpHeaders(2); // Do the HTTP GET request @@ -284,16 +284,16 @@ if (bodySize == 0) { - that.httpClient_.GetBody().clear(); + that.httpClient_.ClearBody(); } else { - that.httpClient_.GetBody().assign(bodyData, bodySize); + that.httpClient_.AssignBody(bodyData, bodySize); } } else { - that.httpClient_.GetBody().clear(); + that.httpClient_.ClearBody(); } // Do the HTTP POST/PUT request @@ -342,7 +342,7 @@ const char* url = lua_tostring(state, 1); that.httpClient_.SetMethod(HttpMethod_Delete); that.httpClient_.SetUrl(url); - that.httpClient_.GetBody().clear(); + that.httpClient_.ClearBody(); that.SetHttpHeaders(2); // Do the HTTP DELETE request diff -r e915102093de -r 9804d6490872 OrthancServer/Plugins/Engine/OrthancPlugins.cpp --- a/OrthancServer/Plugins/Engine/OrthancPlugins.cpp Fri Apr 30 10:09:50 2021 +0200 +++ b/OrthancServer/Plugins/Engine/OrthancPlugins.cpp Tue May 04 10:57:42 2021 +0200 @@ -3332,12 +3332,12 @@ case OrthancPluginHttpMethod_Post: client.SetMethod(HttpMethod_Post); - client.GetBody().assign(reinterpret_cast(parameters.body), parameters.bodySize); + client.SetExternalBody(parameters.body, parameters.bodySize); break; case OrthancPluginHttpMethod_Put: client.SetMethod(HttpMethod_Put); - client.GetBody().assign(reinterpret_cast(parameters.body), parameters.bodySize); + client.SetExternalBody(parameters.body, parameters.bodySize); break; case OrthancPluginHttpMethod_Delete: @@ -3446,7 +3446,7 @@ if (p.method == OrthancPluginHttpMethod_Post || p.method == OrthancPluginHttpMethod_Put) { - client.GetBody().assign(reinterpret_cast(p.body), p.bodySize); + client.SetExternalBody(p.body, p.bodySize); } SetupHttpClient(client, p); @@ -3645,12 +3645,12 @@ case OrthancPluginHttpMethod_Post: client.SetMethod(HttpMethod_Post); - client.GetBody().assign(reinterpret_cast(p.body), p.bodySize); + client.SetExternalBody(p.body, p.bodySize); break; case OrthancPluginHttpMethod_Put: client.SetMethod(HttpMethod_Put); - client.GetBody().assign(reinterpret_cast(p.body), p.bodySize); + client.SetExternalBody(p.body, p.bodySize); break; case OrthancPluginHttpMethod_Delete: diff -r e915102093de -r 9804d6490872 OrthancServer/Sources/OrthancRestApi/OrthancRestModalities.cpp --- a/OrthancServer/Sources/OrthancRestApi/OrthancRestModalities.cpp Fri Apr 30 10:09:50 2021 +0200 +++ b/OrthancServer/Sources/OrthancRestApi/OrthancRestModalities.cpp Tue May 04 10:57:42 2021 +0200 @@ -1862,7 +1862,7 @@ HttpClient client(info, "instances"); client.SetMethod(HttpMethod_Post); client.AddHeader("Expect", ""); - client.GetBody().assign(reinterpret_cast(call.GetBodyData()), call.GetBodySize()); + client.SetExternalBody(call.GetBodyData(), call.GetBodySize()); Json::Value answer; if (client.Apply(answer)) diff -r e915102093de -r 9804d6490872 OrthancServer/Sources/ServerJobs/Operations/StorePeerOperation.cpp --- a/OrthancServer/Sources/ServerJobs/Operations/StorePeerOperation.cpp Fri Apr 30 10:09:50 2021 +0200 +++ b/OrthancServer/Sources/ServerJobs/Operations/StorePeerOperation.cpp Tue May 04 10:57:42 2021 +0200 @@ -63,8 +63,12 @@ try { - instance.ReadDicom(client.GetBody()); + // Lifetime of "body" must exceed the call to "client.Apply()" because of "SetExternalBody()" + std::string body; + instance.ReadDicom(body); + client.SetExternalBody(body); // Avoids a memcpy() + std::string answer; if (!client.Apply(answer)) { diff -r e915102093de -r 9804d6490872 OrthancServer/Sources/ServerJobs/OrthancPeerStoreJob.cpp --- a/OrthancServer/Sources/ServerJobs/OrthancPeerStoreJob.cpp Fri Apr 30 10:09:50 2021 +0200 +++ b/OrthancServer/Sources/ServerJobs/OrthancPeerStoreJob.cpp Tue May 04 10:57:42 2021 +0200 @@ -63,6 +63,7 @@ LOG(INFO) << "Sending instance " << instance << " to peer \"" << peer_.GetUrl() << "\""; + // Lifetime of "body" must exceed the call to "client_->Apply()" because of "SetExternalBody()" std::string body; try @@ -99,19 +100,24 @@ return false; } + // Lifetime of "compressedBody" must exceed the call to "client_->Apply()" because of "SetExternalBody()" + std::string compressedBody; + if (compress_) { GzipCompressor compressor; compressor.SetCompressionLevel(9); // Max compression level - IBufferCompressor::Compress(client_->GetBody(), compressor, body); + IBufferCompressor::Compress(compressedBody, compressor, body); + + client_->SetExternalBody(compressedBody); + size_ += compressedBody.size(); } else { - client_->GetBody().swap(body); + client_->SetExternalBody(body); + size_ += body.size(); } - size_ += client_->GetBody().size(); - std::string answer; if (client_->Apply(answer)) {