Mercurial > hg > orthanc-dicomweb
changeset 582:d32e224e1758
merge
author | Alain Mazy <am@osimis.io> |
---|---|
date | Wed, 28 Jun 2023 14:38:47 +0200 |
parents | 0354c92bbc63 (diff) 1a5b52c8e785 (current diff) |
children | 13d932c8a743 |
files | NEWS Plugin/WadoRs.cpp |
diffstat | 4 files changed, 271 insertions(+), 27 deletions(-) [+] |
line wrap: on
line diff
--- a/CMakeLists.txt Wed Jun 28 11:43:10 2023 +0200 +++ b/CMakeLists.txt Wed Jun 28 14:38:47 2023 +0200 @@ -100,6 +100,10 @@ include_directories(${CMAKE_SOURCE_DIR}/Resources/Orthanc/Sdk-1.9.2) elseif (ORTHANC_SDK_VERSION STREQUAL "1.11.3") include_directories(${CMAKE_SOURCE_DIR}/Resources/Orthanc/Sdk-1.11.3) + elseif (ORTHANC_SDK_VERSION STREQUAL "framework") + set(tmp ${ORTHANC_FRAMEWORK_ROOT}/../../OrthancServer/Plugins/Include/) + message(${tmp}) + include_directories(${tmp}) else() message(FATAL_ERROR "Unsupported version of the Orthanc plugin SDK: ${ORTHANC_SDK_VERSION}") endif()
--- a/NEWS Wed Jun 28 11:43:10 2023 +0200 +++ b/NEWS Wed Jun 28 14:38:47 2023 +0200 @@ -5,9 +5,6 @@ * Support "X-Forwarded-Host" and "X-Forwarded-Proto" headers to compute BulkDataURI. * Small speed-up the studies/../series/../metadata route when in "MainDicomTags" mode. * Fix issue #216 (Requests fail due to bad parsing of the accept HTTP header (semi-colons)) -* New "Experimental" mode for "SeriesMetadata" that is fetching tags from the DB or - from the storage if they are missing (useful if you recently changed the "ExtraMainDicomTags" - configuration) Version 1.13 (2023-02-03)
--- a/Plugin/WadoRs.cpp Wed Jun 28 11:43:10 2023 +0200 +++ b/Plugin/WadoRs.cpp Wed Jun 28 14:38:47 2023 +0200 @@ -29,9 +29,11 @@ #include <HttpServer/HttpContentNegociation.h> #include <Logging.h> #include <Toolbox.h> +#include <MultiThreading/SharedMessageQueue.h> #include <memory> #include <boost/thread/mutex.hpp> +#include <boost/thread.hpp> static const char* const MAIN_DICOM_TAGS = "MainDicomTags"; static const char* const REQUESTED_TAGS = "RequestedTags"; @@ -1072,9 +1074,6 @@ } - - - static void GetChildrenIdentifiers(std::list<std::string>& target, std::string& resourceDicomUid, Orthanc::ResourceType level, @@ -1126,6 +1125,68 @@ } +typedef std::map<std::string, boost::shared_ptr<Orthanc::DicomMap> > ChildrenMainDicomMaps; + +static void GetChildrenMainDicomTags(ChildrenMainDicomMaps& childrenDicomMaps, + std::string& resourceDicomUid, + Orthanc::ResourceType level, + const std::string& orthancId) +{ + childrenDicomMaps.clear(); + + const char* childrenRoute = NULL; + const char* dicomUidTag = NULL; + std::string uri; + + switch (level) + { + case Orthanc::ResourceType_Study: + uri = "/studies/" + orthancId; + childrenRoute = "series"; + dicomUidTag = "StudyInstanceUID"; + break; + + case Orthanc::ResourceType_Series: + uri = "/series/" + orthancId; + childrenRoute = "instances"; + dicomUidTag = "SeriesInstanceUID"; + break; + + default: + throw Orthanc::OrthancException(Orthanc::ErrorCode_ParameterOutOfRange); + } + + assert(childrenRoute != NULL && dicomUidTag != NULL); + + // get the resource itself + Json::Value resource; + if (OrthancPlugins::RestApiGet(resource, uri, false)) + { + if (resource.type() != Json::objectValue + || !resource.isMember(MAIN_DICOM_TAGS) || !resource[MAIN_DICOM_TAGS].isMember(dicomUidTag)) + { + throw Orthanc::OrthancException(Orthanc::ErrorCode_NetworkProtocol); + } + + resourceDicomUid = resource[MAIN_DICOM_TAGS][dicomUidTag].asString(); + + // get the children resources + Json::Value childResources; + if (OrthancPlugins::RestApiGet(childResources, uri + "/" + childrenRoute + "?expand&full", false)) + { + for (Json::Value::ArrayIndex i = 0; i < childResources.size(); i++) + { + const Json::Value& child = childResources[i]; + Orthanc::DicomMap dicom; + dicom.FromDicomAsJson(child[MAIN_DICOM_TAGS], false /* append */, true /* parseSequences */); + childrenDicomMaps[child["ID"].asString()] = boost::shared_ptr<Orthanc::DicomMap>(dicom.Clone()); + } + + } + + } +} + void RetrieveStudyMetadata(OrthancPluginRestOutput* output, const char* url, @@ -1166,10 +1227,122 @@ } +static const char* EXIT_WORKER_MESSAGE = "exit"; + +class InstanceToLoad : public Orthanc::IDynamicObject +{ +public: + std::string orthancId; + std::string studyInstanceUid; + std::string seriesInstanceUid; + std::string bulkRoot; + boost::mutex& writerMutex; + OrthancPlugins::DicomWebFormatter::HttpWriter& writer; + bool isXml; + OrthancPluginDicomWebBinaryMode mode; + + explicit InstanceToLoad(const std::string& orthancId, const std::string bulkRoot, boost::mutex& writerMutex, OrthancPlugins::DicomWebFormatter::HttpWriter& writer, bool isXml, OrthancPluginDicomWebBinaryMode mode, const std::string& studyInstanceUid, const std::string& seriesInstanceUid) + : orthancId(orthancId), + studyInstanceUid(studyInstanceUid), + seriesInstanceUid(seriesInstanceUid), + bulkRoot(bulkRoot), + writerMutex(writerMutex), + writer(writer), + isXml(isXml), + mode(mode) + {} +}; + +class InstanceWorkerData +{ +public: + std::string id; + Orthanc::SharedMessageQueue* instancesQueue; + std::string wadoBase; + + InstanceWorkerData(const std::string& id, Orthanc::SharedMessageQueue* instancesQueue, const std::string& wadoBase) + : id(id), + instancesQueue(instancesQueue), + wadoBase(wadoBase) + {} +}; + +void InstanceWorkerThread(InstanceWorkerData* data) +{ + size_t instanceCounter = 0; + size_t totalTime1 = 0; + size_t totalTime2 = 0; + size_t totalTime3 = 0; + std::string threadId = data->id; + + while (true) + { + try + { + std::unique_ptr<InstanceToLoad> instanceToLoad(dynamic_cast<InstanceToLoad*>(data->instancesQueue->Dequeue(0))); + + if (instanceToLoad->orthancId == EXIT_WORKER_MESSAGE) + { + if (instanceCounter > 0) + { + LOG(WARNING) << threadId << " i: " << instanceCounter << " " << totalTime1/instanceCounter << " " << totalTime2/instanceCounter << " " << totalTime3/instanceCounter; + } + return; + } + instanceCounter++; + boost::posix_time::ptime start2 = boost::posix_time::microsec_clock::universal_time(); + boost::posix_time::ptime stop2 = boost::posix_time::microsec_clock::universal_time(); + + if (instanceToLoad->bulkRoot == "") // we are not in oneLargeQuery mode -> we must load the instance tags to get the SOPInstanceUID + { + Json::Value instanceResource; + + if (OrthancPlugins::RestApiGet(instanceResource, "/instances/" + instanceToLoad->orthancId, false)) + { + instanceToLoad->bulkRoot = (data->wadoBase + + "studies/" + instanceToLoad->studyInstanceUid + + "/series/" + instanceToLoad->seriesInstanceUid + + "/instances/" + instanceResource["MainDicomTags"]["SOPInstanceUID"].asString() + "/bulk"); + } + } + + std::string content; + std::unique_ptr<OrthancPlugins::DicomInstance> instance; + + try + { + instance.reset(OrthancPlugins::DicomInstance::Load(instanceToLoad->orthancId, OrthancPluginLoadDicomInstanceMode_EmptyPixelData)); + } + catch (Orthanc::OrthancException& e) + { + } + + if (instance.get() != NULL) + { + boost::mutex::scoped_lock lock(instanceToLoad->writerMutex); + instanceToLoad->writer.AddInstance(*instance, instanceToLoad->bulkRoot); + } + + stop2 = boost::posix_time::microsec_clock::universal_time(); //LOG(WARNING) << data->id << " written one instance " << (stop2-start2).total_microseconds() << "us"; + totalTime3 += (stop2-start2).total_microseconds(); + } + catch(...) + { + // ignore errors but don't exit the thread to make sure all workers end correctly + } + } +} + + + void RetrieveSeriesMetadata(OrthancPluginRestOutput* output, const char* url, const OrthancPluginHttpRequest* request) { + boost::posix_time::ptime start = boost::posix_time::microsec_clock::universal_time(); + boost::posix_time::ptime stop = boost::posix_time::microsec_clock::universal_time(); + // stop = boost::posix_time::microsec_clock::universal_time(); LOG(WARNING) << "start " << (stop-start).total_milliseconds(); + bool isXml; AcceptMetadata(request, isXml); @@ -1183,15 +1356,92 @@ if (LocateSeries(output, seriesOrthancId, studyInstanceUid, seriesInstanceUid, request)) { OrthancPlugins::DicomWebFormatter::HttpWriter writer(output, isXml); - std::list<std::string> instances; - std::string seriesDicomUid; // not used + + ChildrenMainDicomMaps instancesDicomMaps; + std::list<std::string> instancesIds; + std::string seriesDicomUid; + + size_t threadCount = 4; + bool oneLargeQuery = false; // we keep this code here for future use once we'll have optimized Orthanc API /series/.../instances?full to minimize the SQL queries + // right now, it is faster to call /instances/..?full in each worker but, later, it should be more efficient with a large SQL query in Orthanc + + if (oneLargeQuery) + { + GetChildrenMainDicomTags(instancesDicomMaps, seriesDicomUid, Orthanc::ResourceType_Series, seriesOrthancId); + } + else + { + GetChildrenIdentifiers(instancesIds, seriesDicomUid, Orthanc::ResourceType_Series, seriesOrthancId); + } - GetChildrenIdentifiers(instances, seriesDicomUid, Orthanc::ResourceType_Series, seriesOrthancId); + if (threadCount > 1) + { + // span a few workers to get the tags from the core and serialize them + Orthanc::SharedMessageQueue instancesQueue; + std::vector<boost::shared_ptr<boost::thread> > instancesWorkers; + boost::mutex writerMutex; + std::vector<boost::shared_ptr<InstanceWorkerData> > instancesWorkersData; + std::string wadoBase = OrthancPlugins::Configuration::GetBasePublicUrl(request); + + for (size_t t; t < threadCount; t++) + { + InstanceWorkerData* threadData = new InstanceWorkerData(boost::lexical_cast<std::string>(t), &instancesQueue, wadoBase); + instancesWorkersData.push_back(boost::shared_ptr<InstanceWorkerData>(threadData)); + instancesWorkers.push_back(boost::shared_ptr<boost::thread>(new boost::thread(InstanceWorkerThread, threadData))); + } + + if (oneLargeQuery) + { + for (ChildrenMainDicomMaps::const_iterator i = instancesDicomMaps.begin(); i != instancesDicomMaps.end(); ++i) + { + std::string bulkRoot = (wadoBase + + "studies/" + studyInstanceUid + + "/series/" + seriesInstanceUid + + "/instances/" + i->second->GetStringValue(Orthanc::DICOM_TAG_SOP_INSTANCE_UID, "", false) + "/bulk"); - for (std::list<std::string>::const_iterator i = instances.begin(); i != instances.end(); ++i) - { - WriteInstanceMetadata(writer, mode, cache, *i, studyInstanceUid, seriesInstanceUid, - OrthancPlugins::Configuration::GetBasePublicUrl(request)); + instancesQueue.Enqueue(new InstanceToLoad(i->first, bulkRoot, writerMutex, writer, isXml, OrthancPluginDicomWebBinaryMode_BulkDataUri, studyInstanceUid, seriesInstanceUid)); + } + } + else + { + for (std::list<std::string>::const_iterator i = instancesIds.begin(); i != instancesIds.end(); ++i) + { + instancesQueue.Enqueue(new InstanceToLoad(*i, "", writerMutex, writer, isXml, OrthancPluginDicomWebBinaryMode_BulkDataUri, studyInstanceUid, seriesInstanceUid)); + } + } + + stop = boost::posix_time::microsec_clock::universal_time(); LOG(WARNING) << "enqueued " << (stop-start).total_milliseconds(); + + // send a dummy "exit" message to all workers such that they stop waiting for messages on the queue + for (size_t i = 0; i < instancesWorkers.size(); i++) + { + instancesQueue.Enqueue(new InstanceToLoad(EXIT_WORKER_MESSAGE, "", writerMutex, writer, isXml, OrthancPluginDicomWebBinaryMode_BulkDataUri, studyInstanceUid, seriesInstanceUid)); + } + + stop = boost::posix_time::microsec_clock::universal_time(); LOG(WARNING) << "waiting " << (stop-start).total_milliseconds(); + + for (size_t i = 0; i < instancesWorkers.size(); i++) + { + if (instancesWorkers[i]->joinable()) + { + instancesWorkers[i]->join(); + } + } + + instancesWorkers.clear(); + } + else + { // old single threaded code + std::list<std::string> instances; + std::string seriesDicomUid; // not used + + GetChildrenIdentifiers(instances, seriesDicomUid, Orthanc::ResourceType_Series, seriesOrthancId); + + for (std::list<std::string>::const_iterator i = instances.begin(); i != instances.end(); ++i) + { + WriteInstanceMetadata(writer, mode, cache, *i, studyInstanceUid, seriesInstanceUid, + OrthancPlugins::Configuration::GetBasePublicUrl(request)); + } } writer.Send();
--- a/TODO Wed Jun 28 11:43:10 2023 +0200 +++ b/TODO Wed Jun 28 14:38:47 2023 +0200 @@ -15,21 +15,14 @@ with a 600 instance series with SQLite - all timings are performed without verbose logs !!!!: - time curl http://localhost:8043/dicom-web/studies/1.2.276.0.7230010.3.1.2.1215942821.4756.1664826045.3529/series/1.2.276.0.7230010.3.1.3.1215942821.4756.1664833048.11984/metadata > /dev/null - -> 0.750 second because the plugin makes 600 calls to /instances/...?full - to retrieve all main dicom tags from the DB (but this also retrieves other - data like attachment info + instances metadata) - - In "Experimental" mode, we are calling this route: - time curl "http://localhost:8043/series/fdd31453-fa62d8cb-b681823c-f294d34c-182bca4b/instances?full&requestedTags=0008,0005;0008,0008;0008,0012;0008,0013;0008,0016;0008,0018;0008,0023;0008,0033;0018,0050;0020,0012;0020,0013;0020,0032;0020,0037;0020,0052;0020,0100;0020,4000;0028,0004;0028,0008;0028,0010;0028,0011;0028,0030;0028,0100;0028,0101;0028,0103;0028,1050;0028,1051;0028,1052;0028,1053;0054,1330" > /dev/null - -> 0.175 second - BUT the total time required by the /metadata route is still around 0.9 second because we spend 600-700ms formating the output !!! (see TODO_OPTI in WadoRs.cpp) - -> 0.835 second in "Experimental" mode (10% slower if tags are actually in DB of course !) - -> 0.700 second in "MainDicomTags" mode (10% faster) - FURTHER WORK: - - In the plugin SDK, we should probably have a method to convert DicomAsJson_Full to DicomWebJson without creating a ParsedDicomFile in between. - - Or in the core: implement /series/.../instances?dicom-web-json (the core has the primitives to write dicom-web-json) - - Even if we optimize the DB, the cost for the response serialization will still be very high and that's likely the first thing to work on !!! - + -> 883ms in Full mode + -> 565ms in MainDicomTags mode + -> 545ms in Full mode with 1 worker + -> 335ms in Full mode with 2 workers + -> 267ms in Full mode with 3 workers + -> 270ms in Full mode with 4 workers + -> 270ms in Full mode with 8 workers - note that all measurements have been performed on a DB with a single series ! We should repeat