# HG changeset patch
# User Sebastien Jodogne <s.jodogne@gmail.com>
# Date 1587049117 -7200
# Node ID be7df7fe3d80c7f084cc1cfc4617cbbb8f5be697
# Parent  e7003b2203a72d4c8017fc36168e03403c320bba
avoid one memcpy of the DICOM buffer on "POST /instances"

diff -r e7003b2203a7 -r be7df7fe3d80 NEWS
--- a/NEWS	Wed Apr 15 22:17:42 2020 +0200
+++ b/NEWS	Thu Apr 16 16:58:37 2020 +0200
@@ -20,6 +20,7 @@
 * Fix signature of "OrthancPluginRegisterStorageCommitmentScpCallback()" in plugins SDK
 * Error reporting on failure while initializing SSL
 * Fix unit test ParsedDicomFile.ToJsonFlags2 on big-endian architectures
+* Avoid one memcpy of the DICOM buffer on "POST /instances"
 * Upgraded dependencies for static builds (notably on Windows):
   - civetweb 1.12
   - openssl 1.1.1f
diff -r e7003b2203a7 -r be7df7fe3d80 OrthancServer/DicomInstanceToStore.cpp
--- a/OrthancServer/DicomInstanceToStore.cpp	Wed Apr 15 22:17:42 2020 +0200
+++ b/OrthancServer/DicomInstanceToStore.cpp	Thu Apr 16 16:58:37 2020 +0200
@@ -150,18 +150,28 @@
   {
   public:
     DicomInstanceOrigin                  origin_;
-    SmartContainer<std::string>          buffer_;
+    bool                                 hasBuffer_;
+    std::unique_ptr<std::string>         ownBuffer_;
+    const void*                          bufferData_;
+    size_t                               bufferSize_;
     SmartContainer<ParsedDicomFile>      parsed_;
     SmartContainer<DicomMap>             summary_;
     SmartContainer<Json::Value>          json_;
     MetadataMap                          metadata_;
 
+    PImpl() :
+      hasBuffer_(false),
+      bufferData_(NULL),
+      bufferSize_(0)
+    {
+    }
+
   private:
     std::unique_ptr<DicomInstanceHasher>  hasher_;
 
     void ComputeMissingInformation()
     {
-      if (buffer_.HasContent() &&
+      if (hasBuffer_ &&
           summary_.HasContent() &&
           json_.HasContent())
       {
@@ -169,7 +179,7 @@
         return; 
       }
     
-      if (!buffer_.HasContent())
+      if (!hasBuffer_)
       {
         if (!parsed_.HasContent())
         {
@@ -186,13 +196,15 @@
         }
 
         // Serialize the parsed DICOM file
-        buffer_.Allocate();
-        if (!FromDcmtkBridge::SaveToMemoryBuffer(buffer_.GetContent(), 
+        ownBuffer_.reset(new std::string);
+        if (!FromDcmtkBridge::SaveToMemoryBuffer(*ownBuffer_,
                                                  *parsed_.GetContent().GetDcmtkObject().getDataset()))
         {
           throw OrthancException(ErrorCode_InternalError,
                                  "Unable to serialize a DICOM file to a memory buffer");
         }
+
+        hasBuffer_ = true;
       }
 
       if (summary_.HasContent() &&
@@ -205,9 +217,17 @@
       // memory buffer, but that its summary or its JSON version is
       // missing
 
+      assert(hasBuffer_);
       if (!parsed_.HasContent())
       {
-        parsed_.TakeOwnership(new ParsedDicomFile(buffer_.GetConstContent()));
+        if (ownBuffer_.get() != NULL)
+        {
+          parsed_.TakeOwnership(new ParsedDicomFile(*ownBuffer_));
+        }
+        else
+        {
+          parsed_.TakeOwnership(new ParsedDicomFile(bufferData_, bufferSize_));
+        }
       }
 
       // At this point, we have parsed the DICOM file
@@ -232,22 +252,38 @@
 
 
   public:
-    const char* GetBufferData()
+    void SetBuffer(const void* data,
+                   size_t size)
+    {
+      ownBuffer_.reset(NULL);
+      bufferData_ = data;
+      bufferSize_ = size;
+      hasBuffer_ = true;
+    }
+    
+    const void* GetBufferData()
     {
       ComputeMissingInformation();
-    
-      if (!buffer_.HasContent())
+
+      if (!hasBuffer_)
       {
         throw OrthancException(ErrorCode_InternalError);
       }
 
-      if (buffer_.GetConstContent().size() == 0)
+      if (ownBuffer_.get() != NULL)
       {
-        return NULL;
+        if (ownBuffer_->empty())
+        {
+          return NULL;
+        }
+        else
+        {
+          return ownBuffer_->c_str();
+        }
       }
       else
       {
-        return buffer_.GetConstContent().c_str();
+        return bufferData_;
       }
     }
 
@@ -256,12 +292,19 @@
     {
       ComputeMissingInformation();
     
-      if (!buffer_.HasContent())
+      if (!hasBuffer_)
       {
         throw OrthancException(ErrorCode_InternalError);
       }
 
-      return buffer_.GetConstContent().size();
+      if (ownBuffer_.get() != NULL)
+      {
+        return ownBuffer_->size();
+      }
+      else
+      {
+        return bufferSize_;
+      }
     }
 
 
@@ -347,9 +390,10 @@
   }
 
     
-  void DicomInstanceToStore::SetBuffer(const std::string& dicom)
+  void DicomInstanceToStore::SetBuffer(const void* dicom,
+                                       size_t size)
   {
-    pimpl_->buffer_.SetConstReference(dicom);
+    pimpl_->SetBuffer(dicom, size);
   }
 
 
@@ -391,7 +435,7 @@
   }
 
 
-  const char* DicomInstanceToStore::GetBufferData()
+  const void* DicomInstanceToStore::GetBufferData()
   {
     return pimpl_->GetBufferData();
   }
diff -r e7003b2203a7 -r be7df7fe3d80 OrthancServer/DicomInstanceToStore.h
--- a/OrthancServer/DicomInstanceToStore.h	Wed Apr 15 22:17:42 2020 +0200
+++ b/OrthancServer/DicomInstanceToStore.h	Thu Apr 16 16:58:37 2020 +0200
@@ -59,8 +59,11 @@
     void SetOrigin(const DicomInstanceOrigin& origin);
     
     const DicomInstanceOrigin& GetOrigin() const;
-    
-    void SetBuffer(const std::string& dicom);
+
+    // WARNING: The buffer is not copied, it must not be removed as
+    // long as the "DicomInstanceToStore" object is alive
+    void SetBuffer(const void* dicom,
+                   size_t size);
 
     void SetParsedDicomFile(ParsedDicomFile& parsed);
 
@@ -76,7 +79,7 @@
                      MetadataType metadata,
                      const std::string& value);
 
-    const char* GetBufferData();
+    const void* GetBufferData();
 
     size_t GetBufferSize();
 
diff -r e7003b2203a7 -r be7df7fe3d80 OrthancServer/OrthancRestApi/OrthancRestApi.cpp
--- a/OrthancServer/OrthancRestApi/OrthancRestApi.cpp	Wed Apr 15 22:17:42 2020 +0200
+++ b/OrthancServer/OrthancRestApi/OrthancRestApi.cpp	Thu Apr 16 16:58:37 2020 +0200
@@ -121,22 +121,23 @@
                              "Received an empty DICOM file");
     }
 
+    // The lifetime of "dicom" must be longer than "toStore", as the
+    // latter can possibly store a reference to the former (*)
     std::string dicom;
 
+    DicomInstanceToStore toStore;
+    toStore.SetOrigin(DicomInstanceOrigin::FromRest(call));
+
     if (boost::iequals(call.GetHttpHeader("content-encoding", ""), "gzip"))
     {
       GzipCompressor compressor;
       compressor.Uncompress(dicom, call.GetBodyData(), call.GetBodySize());
+      toStore.SetBuffer(dicom.c_str(), dicom.size());  // (*)
     }
     else
     {
-      // TODO Remove unneccessary memcpy
-      call.BodyToString(dicom);
-    }
-
-    DicomInstanceToStore toStore;
-    toStore.SetOrigin(DicomInstanceOrigin::FromRest(call));
-    toStore.SetBuffer(dicom);
+      toStore.SetBuffer(call.GetBodyData(), call.GetBodySize());
+    }    
 
     std::string publicId;
     StoreStatus status = context.Store(publicId, toStore);
diff -r e7003b2203a7 -r be7df7fe3d80 OrthancServer/main.cpp
--- a/OrthancServer/main.cpp	Wed Apr 15 22:17:42 2020 +0200
+++ b/OrthancServer/main.cpp	Thu Apr 16 16:58:37 2020 +0200
@@ -82,7 +82,7 @@
       DicomInstanceToStore toStore;
       toStore.SetOrigin(DicomInstanceOrigin::FromDicomProtocol
                         (remoteIp.c_str(), remoteAet.c_str(), calledAet.c_str()));
-      toStore.SetBuffer(dicomFile);
+      toStore.SetBuffer(dicomFile.c_str(), dicomFile.size());
       toStore.SetSummary(dicomSummary);
       toStore.SetJson(dicomJson);