Mercurial > hg > orthanc
changeset 3841:be7df7fe3d80
avoid one memcpy of the DICOM buffer on "POST /instances"
author | Sebastien Jodogne <s.jodogne@gmail.com> |
---|---|
date | Thu, 16 Apr 2020 16:58:37 +0200 |
parents | e7003b2203a7 |
children | bdbe12aba99f |
files | NEWS OrthancServer/DicomInstanceToStore.cpp OrthancServer/DicomInstanceToStore.h OrthancServer/OrthancRestApi/OrthancRestApi.cpp OrthancServer/main.cpp |
diffstat | 5 files changed, 77 insertions(+), 28 deletions(-) [+] |
line wrap: on
line diff
--- 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
--- 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(); }
--- 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();
--- 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);
--- 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);