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