changeset 4650:9804d6490872

Reduced memory consumption of HTTP/REST plugins calls on POST/PUT if chunked transfer is disabled
author Sebastien Jodogne <s.jodogne@gmail.com>
date Tue, 04 May 2021 10:57:42 +0200
parents e915102093de
children 365f51fae413
files NEWS OrthancFramework/Sources/HttpClient.cpp OrthancFramework/Sources/HttpClient.h OrthancFramework/Sources/Lua/LuaContext.cpp OrthancServer/Plugins/Engine/OrthancPlugins.cpp OrthancServer/Sources/OrthancRestApi/OrthancRestModalities.cpp OrthancServer/Sources/ServerJobs/Operations/StorePeerOperation.cpp OrthancServer/Sources/ServerJobs/OrthancPeerStoreJob.cpp
diffstat 8 files changed, 93 insertions(+), 29 deletions(-) [+]
line wrap: on
line diff
--- 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)
--- 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<const char*>(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()));
--- 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);
--- 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
--- 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<const char*>(parameters.body), parameters.bodySize);
+        client.SetExternalBody(parameters.body, parameters.bodySize);
         break;
 
       case OrthancPluginHttpMethod_Put:
         client.SetMethod(HttpMethod_Put);
-        client.GetBody().assign(reinterpret_cast<const char*>(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<const char*>(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<const char*>(p.body), p.bodySize);
+        client.SetExternalBody(p.body, p.bodySize);
         break;
 
       case OrthancPluginHttpMethod_Put:
         client.SetMethod(HttpMethod_Put);
-        client.GetBody().assign(reinterpret_cast<const char*>(p.body), p.bodySize);
+        client.SetExternalBody(p.body, p.bodySize);
         break;
 
       case OrthancPluginHttpMethod_Delete:
--- 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<const char*>(call.GetBodyData()), call.GetBodySize());
+    client.SetExternalBody(call.GetBodyData(), call.GetBodySize());
 
     Json::Value answer;
     if (client.Apply(answer))
--- 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))
       {
--- 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))
     {