# HG changeset patch
# User Sebastien Jodogne <s.jodogne@gmail.com>
# Date 1572976146 -3600
# Node ID a8bf817568392d8104f507d83bd0b1a952e99753
# Parent  383aa2a7d4261b9a8e8c94b9ce5f0f60e420de3d
unsuccessful attempt to cache ParseDicomFileCommand

diff -r 383aa2a7d426 -r a8bf81756839 Framework/Oracle/GenericOracleRunner.cpp
--- a/Framework/Oracle/GenericOracleRunner.cpp	Mon Nov 04 15:54:57 2019 +0100
+++ b/Framework/Oracle/GenericOracleRunner.cpp	Tue Nov 05 18:49:06 2019 +0100
@@ -226,8 +226,8 @@
 
 
 #if ORTHANC_ENABLE_DCMTK == 1
-  static IMessage* Execute(const std::string& root,
-                           const ParseDicomFileCommand& command)
+  static ParseDicomFileCommand::SuccessMessage* Execute(const std::string& root,
+                                                        const ParseDicomFileCommand& command)
   {
     std::string path = GetPath(root, command.GetPath());
 
@@ -243,13 +243,13 @@
     {
       throw Orthanc::OrthancException(Orthanc::ErrorCode_NotEnoughMemory);
     }
-    
-    DcmFileFormat f;
+
+    DcmFileFormat dicom;
     bool ok;
 
     if (command.IsPixelDataIncluded())
     {
-      ok = f.loadFile(path.c_str()).good();
+      ok = dicom.loadFile(path.c_str()).good();
     }
     else
     {
@@ -260,17 +260,20 @@
       static const DcmTagKey STOP = DCM_PixelData;
       //static const DcmTagKey STOP(0x3007, 0x0000);
 
-      ok = f.loadFileUntilTag(path.c_str(), EXS_Unknown, EGL_noChange,
-                              DCM_MaxReadLength, ERM_autoDetect, STOP).good();
+      ok = dicom.loadFileUntilTag(path.c_str(), EXS_Unknown, EGL_noChange,
+                                  DCM_MaxReadLength, ERM_autoDetect, STOP).good();
 #else
       // The primitive "loadFileUntilTag" was introduced in DCMTK 3.6.2
-      ok = f.loadFile(path.c_str()).good();
+      ok = dicom.loadFile(path.c_str()).good();
 #endif
     }
 
+    printf("Reading %s\n", path.c_str());
+
     if (ok)
     {
-      return new ParseDicomFileCommand::SuccessMessage(command, f, static_cast<size_t>(fileSize));
+      return new ParseDicomFileCommand::SuccessMessage
+        (command, dicom, static_cast<size_t>(fileSize), command.IsPixelDataIncluded());
     }
     else
     {
@@ -278,6 +281,48 @@
                                       "Cannot parse file: " + path);
     }
   }
+
+
+  static ParseDicomFileCommand::SuccessMessage* Execute(boost::shared_ptr<ParsedDicomFileCache> cache,
+                                                        const std::string& root,
+                                                        const ParseDicomFileCommand& command)
+  {
+#if 0
+    // The code to use the cache is buggy in multithreaded environments => TODO FIX
+    if (cache.get())
+    {
+      {
+        ParsedDicomFileCache::Reader reader(*cache, command.GetPath());
+        if (reader.IsValid() &&
+            (!command.IsPixelDataIncluded() ||
+             reader.HasPixelData()))
+        {
+          // Reuse the DICOM file from the cache
+          return new ParseDicomFileCommand::SuccessMessage(
+            command, reader.GetDicom(), reader.GetFileSize(), reader.HasPixelData());
+        }
+      }
+      
+      // Not in the cache, first read and parse the DICOM file
+      std::auto_ptr<ParseDicomFileCommand::SuccessMessage> message(Execute(root, command));
+
+      // Secondly, store it into the cache for future use
+      assert(&message->GetOrigin() == &command);
+      cache->Acquire(message->GetOrigin().GetPath(), message->GetDicom(),
+                     message->GetFileSize(), message->HasPixelData());
+
+      return message.release();
+    }
+    else
+    {
+      // No cache available
+      return Execute(root, command);
+    }
+#else
+    return Execute(root, command);
+#endif
+  }
+  
 #endif
 
 
@@ -311,7 +356,8 @@
 
         case IOracleCommand::Type_ParseDicomFile:
 #if ORTHANC_ENABLE_DCMTK == 1
-          return Execute(rootDirectory_, dynamic_cast<const ParseDicomFileCommand&>(command));
+          return Execute(dicomCache_, rootDirectory_,
+                         dynamic_cast<const ParseDicomFileCommand&>(command));
 #else
           throw Orthanc::OrthancException(Orthanc::ErrorCode_NotImplemented,
                                           "DCMTK must be enabled to parse DICOM files");
diff -r 383aa2a7d426 -r a8bf81756839 Framework/Oracle/GenericOracleRunner.h
--- a/Framework/Oracle/GenericOracleRunner.h	Mon Nov 04 15:54:57 2019 +0100
+++ b/Framework/Oracle/GenericOracleRunner.h	Tue Nov 05 18:49:06 2019 +0100
@@ -23,6 +23,15 @@
 
 #include "IOracleRunner.h"
 
+#if !defined(ORTHANC_ENABLE_DCMTK)
+#  error The macro ORTHANC_ENABLE_DCMTK must be defined
+#endif
+
+#if ORTHANC_ENABLE_DCMTK == 1
+#  include "../Toolbox/ParsedDicomFileCache.h"
+#endif
+
+
 #include <Core/Enumerations.h>  // For ORTHANC_OVERRIDE
 #include <Core/WebServiceParameters.h>
 
@@ -34,9 +43,16 @@
     Orthanc::WebServiceParameters  orthanc_;
     std::string                    rootDirectory_;
 
+#if ORTHANC_ENABLE_DCMTK == 1
+    boost::shared_ptr<ParsedDicomFileCache>  dicomCache_;
+#endif
+
   public:
-    GenericOracleRunner() :
-      rootDirectory_(".")
+    GenericOracleRunner()
+      : rootDirectory_(".")
+#if ORTHANC_ENABLE_DCMTK == 1
+      , dicomCache_(NULL)
+#endif
     {
     }
 
@@ -60,6 +76,13 @@
       return rootDirectory_;
     }
 
+#if ORTHANC_ENABLE_DCMTK == 1
+    void SetDicomCache(boost::shared_ptr<ParsedDicomFileCache> cache)
+    {
+      dicomCache_ = cache;
+    }
+#endif
+
     virtual IMessage* Run(IOracleCommand& command) ORTHANC_OVERRIDE;
   };
 }
diff -r 383aa2a7d426 -r a8bf81756839 Framework/Oracle/ParseDicomFileCommand.cpp
--- a/Framework/Oracle/ParseDicomFileCommand.cpp	Mon Nov 04 15:54:57 2019 +0100
+++ b/Framework/Oracle/ParseDicomFileCommand.cpp	Tue Nov 05 18:49:06 2019 +0100
@@ -27,14 +27,8 @@
 
 namespace OrthancStone
 {
-  ParseDicomFileCommand::SuccessMessage::SuccessMessage(const ParseDicomFileCommand& command,
-                                                        DcmFileFormat& content,
-                                                        size_t fileSize) :
-    OriginMessage(command),
-    fileSize_(fileSize)
+  void ParseDicomFileCommand::SuccessMessage::Setup()
   {
-    dicom_.reset(new Orthanc::ParsedDicomFile(content));
-
     if (!dicom_->GetTagValue(sopInstanceUid_, Orthanc::DICOM_TAG_SOP_INSTANCE_UID))
     {
       throw Orthanc::OrthancException(Orthanc::ErrorCode_BadFileFormat,
@@ -43,10 +37,29 @@
   }
 
 
-  boost::shared_ptr<Orthanc::ParsedDicomFile> ParseDicomFileCommand::SuccessMessage::GetDicom() const
+  ParseDicomFileCommand::SuccessMessage::SuccessMessage(const ParseDicomFileCommand& command,
+                                                        DcmFileFormat& dicom,
+                                                        size_t fileSize,
+                                                        bool hasPixelData) :
+    OriginMessage(command),
+    fileSize_(fileSize),
+    hasPixelData_(hasPixelData)
   {
-    assert(dicom_.get() != NULL);
-    return dicom_;
+    dicom_.reset(new Orthanc::ParsedDicomFile(dicom));
+    Setup();
+  }
+
+
+  ParseDicomFileCommand::SuccessMessage::SuccessMessage(const ParseDicomFileCommand& command,
+                                                        boost::shared_ptr<Orthanc::ParsedDicomFile> dicom,
+                                                        size_t fileSize,
+                                                        bool hasPixelData) :
+    OriginMessage(command),
+    dicom_(dicom),
+    fileSize_(fileSize),
+    hasPixelData_(hasPixelData)
+  {
+    Setup();
   }
 
 
diff -r 383aa2a7d426 -r a8bf81756839 Framework/Oracle/ParseDicomFileCommand.h
--- a/Framework/Oracle/ParseDicomFileCommand.h	Mon Nov 04 15:54:57 2019 +0100
+++ b/Framework/Oracle/ParseDicomFileCommand.h	Tue Nov 05 18:49:06 2019 +0100
@@ -48,24 +48,41 @@
     private:
       boost::shared_ptr<Orthanc::ParsedDicomFile>  dicom_;
       size_t                                       fileSize_;
+      bool                                         hasPixelData_;
       std::string                                  sopInstanceUid_;
 
+      void Setup();
+      
     public:
       SuccessMessage(const ParseDicomFileCommand& command,
-                     DcmFileFormat& content,
-                     size_t fileSize);
+                     DcmFileFormat& dicom,
+                     size_t fileSize,
+                     bool hasPixelData);
+      
+      SuccessMessage(const ParseDicomFileCommand& command,
+                     boost::shared_ptr<Orthanc::ParsedDicomFile> dicom,
+                     size_t fileSize,
+                     bool hasPixelData);
+      
+      boost::shared_ptr<Orthanc::ParsedDicomFile> GetDicom() const
+      {
+        return dicom_;
+      }
 
       size_t GetFileSize() const
       {
         return fileSize_;
       }
+
+      bool HasPixelData() const
+      {
+        return hasPixelData_;
+      }
       
       const std::string& GetSopInstanceUid() const
       {
         return sopInstanceUid_;
       }
-      
-      boost::shared_ptr<Orthanc::ParsedDicomFile> GetDicom() const;
     };
 
   private:
diff -r 383aa2a7d426 -r a8bf81756839 Framework/Oracle/ThreadedOracle.cpp
--- a/Framework/Oracle/ThreadedOracle.cpp	Mon Nov 04 15:54:57 2019 +0100
+++ b/Framework/Oracle/ThreadedOracle.cpp	Tue Nov 05 18:49:06 2019 +0100
@@ -181,6 +181,11 @@
           boost::mutex::scoped_lock lock(mutex_);
           runner.SetOrthanc(orthanc_);
           runner.SetRootDirectory(rootDirectory_);
+
+          if (dicomCache_)
+          {
+            runner.SetDicomCache(dicomCache_);
+          }
         }
         
         std::auto_ptr<IMessage> message(runner.Run(item.GetCommand()));
@@ -355,6 +360,31 @@
   }
 
 
+  void ThreadedOracle::SetDicomCacheSize(size_t size)
+  {
+#if ORTHANC_ENABLE_DCMTK == 1
+    boost::mutex::scoped_lock lock(mutex_);
+
+    if (state_ != State_Setup)
+    {
+      LOG(ERROR) << "ThreadedOracle::SetDicomCacheSize(): (state_ != State_Setup)";
+      throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls);
+    }
+    else
+    {
+      if (size == 0)
+      {
+        dicomCache_.reset();
+      }
+      else
+      {
+        dicomCache_.reset(new ParsedDicomFileCache(size));
+      }
+    }
+#endif
+  }
+
+
   void ThreadedOracle::Start()
   {
     boost::mutex::scoped_lock lock(mutex_);
diff -r 383aa2a7d426 -r a8bf81756839 Framework/Oracle/ThreadedOracle.h
--- a/Framework/Oracle/ThreadedOracle.h	Mon Nov 04 15:54:57 2019 +0100
+++ b/Framework/Oracle/ThreadedOracle.h	Tue Nov 05 18:49:06 2019 +0100
@@ -25,10 +25,18 @@
 #  error The macro ORTHANC_ENABLE_THREADS must be defined
 #endif
 
+#if !defined(ORTHANC_ENABLE_DCMTK)
+#  error The macro ORTHANC_ENABLE_DCMTK must be defined
+#endif
+
 #if ORTHANC_ENABLE_THREADS != 1
 #  error This file can only compiled for native targets
 #endif
 
+#if ORTHANC_ENABLE_DCMTK == 1
+#  include "../Toolbox/ParsedDicomFileCache.h"
+#endif
+
 #include "IOracle.h"
 #include "GenericOracleRunner.h"
 #include "../Messages/IMessageEmitter.h"
@@ -62,6 +70,10 @@
     boost::thread                        sleepingWorker_;
     unsigned int                         sleepingTimeResolution_;
 
+#if ORTHANC_ENABLE_DCMTK == 1
+    boost::shared_ptr<ParsedDicomFileCache>  dicomCache_;
+#endif
+    
     void Step();
 
     static void Worker(ThreadedOracle* that);
@@ -83,6 +95,8 @@
 
     void SetSleepingTimeResolution(unsigned int milliseconds);
 
+    void SetDicomCacheSize(size_t size);
+
     void Start();
 
     void Stop()
diff -r 383aa2a7d426 -r a8bf81756839 Framework/Toolbox/ParsedDicomFileCache.cpp
--- a/Framework/Toolbox/ParsedDicomFileCache.cpp	Mon Nov 04 15:54:57 2019 +0100
+++ b/Framework/Toolbox/ParsedDicomFileCache.cpp	Tue Nov 05 18:49:06 2019 +0100
@@ -26,44 +26,116 @@
   class ParsedDicomFileCache::Item : public Orthanc::ICacheable
   {
   private:
-    std::auto_ptr<Orthanc::ParsedDicomFile>  dicom_;
-    size_t                                   fileSize_;
-
+    boost::mutex                                 mutex_;
+    boost::shared_ptr<Orthanc::ParsedDicomFile>  dicom_;
+    size_t                                       fileSize_;
+    bool                                         hasPixelData_;
+    
   public:
-    Item(Orthanc::ParsedDicomFile* dicom,
-         size_t fileSize) :
+    Item(boost::shared_ptr<Orthanc::ParsedDicomFile> dicom,
+         size_t fileSize,
+         bool hasPixelData) :
       dicom_(dicom),
-      fileSize_(fileSize)
+      fileSize_(fileSize),
+      hasPixelData_(hasPixelData)
     {
       if (dicom == NULL)
       {
         throw Orthanc::OrthancException(Orthanc::ErrorCode_NullPointer);
       }
-    }           
+    }
+
+    boost::mutex& GetMutex()
+    {
+      return mutex_;
+    }
            
     virtual size_t GetMemoryUsage() const
     {
       return fileSize_;
     }
 
-    const Orthanc::ParsedDicomFile& GetDicom() const
+    boost::shared_ptr<Orthanc::ParsedDicomFile> GetDicom() const
     {
       assert(dicom_.get() != NULL);
-      return *dicom_;
+      return dicom_;
+    }
+
+    bool HasPixelData() const
+    {
+      return hasPixelData_;
     }
   };
     
 
-  void ParsedDicomFileCache::Acquire(const std::string& sopInstanceUid,
-                                     Orthanc::ParsedDicomFile* dicom,
-                                     size_t fileSize)
+  void ParsedDicomFileCache::Acquire(const std::string& path,
+                                     boost::shared_ptr<Orthanc::ParsedDicomFile> dicom,
+                                     size_t fileSize,
+                                     bool hasPixelData)
   {
-    cache_.Acquire(sopInstanceUid, new Item(dicom, fileSize));
+    cache_.Acquire(path, new Item(dicom, fileSize, hasPixelData));
   }
 
   
-  const Orthanc::ParsedDicomFile& ParsedDicomFileCache::Reader::GetDicom() const
+  ParsedDicomFileCache::Reader::Reader(ParsedDicomFileCache& cache,
+                                       const std::string& path) :
+    reader_(cache.cache_, path)
+  {
+    if (reader_.IsValid())
+    {
+      /**
+       * The "Orthanc::MemoryObjectCache" uses readers/writers. The
+       * "Reader" subclass of the cache locks as a reader. This means
+       * that multiple threads can still access the underlying
+       * "ParsedDicomFile" object, which is not supported by DCMTK. We
+       * thus protect the DCMTK object by a simple mutex.
+       **/
+      
+      item_ = &dynamic_cast<Item&>(reader_.GetValue());
+      lock_.reset(new boost::mutex::scoped_lock(item_->GetMutex()));
+    }
+    else
+    {
+      item_ = NULL;
+    }
+  }
+
+
+  bool ParsedDicomFileCache::Reader::HasPixelData() const
   {
-    return dynamic_cast<const Item&>(reader_.GetValue()).GetDicom();
+    if (item_ == NULL)
+    {
+      throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls);
+    }
+    else
+    {
+      return item_->HasPixelData();
+    }
+  }
+
+  
+  boost::shared_ptr<Orthanc::ParsedDicomFile> ParsedDicomFileCache::Reader::GetDicom() const
+  {
+    if (item_ == NULL)
+    {
+      throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls);
+    }
+    else
+    {
+      return item_->GetDicom();
+    }
+  }
+
+  
+  size_t ParsedDicomFileCache::Reader::GetFileSize() const
+  {
+    if (item_ == NULL)
+    {
+      throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls);
+    }
+    else
+    {
+      return item_->GetMemoryUsage();
+    }
   }
 }
diff -r 383aa2a7d426 -r a8bf81756839 Framework/Toolbox/ParsedDicomFileCache.h
--- a/Framework/Toolbox/ParsedDicomFileCache.h	Mon Nov 04 15:54:57 2019 +0100
+++ b/Framework/Toolbox/ParsedDicomFileCache.h	Tue Nov 05 18:49:06 2019 +0100
@@ -24,6 +24,8 @@
 #include <Core/Cache/MemoryObjectCache.h>
 #include <Core/DicomParsing/ParsedDicomFile.h>
 
+#include <boost/shared_ptr.hpp>
+
 namespace OrthancStone
 {
   class ParsedDicomFileCache : public boost::noncopyable
@@ -39,28 +41,32 @@
       cache_.SetMaximumSize(size);
     }
     
-    void Acquire(const std::string& sopInstanceUid,
-                 Orthanc::ParsedDicomFile* dicom,  // Takes ownership
-                 size_t fileSize);
+    void Acquire(const std::string& path,
+                 boost::shared_ptr<Orthanc::ParsedDicomFile> dicom,
+                 size_t fileSize,
+                 bool hasPixelData);
 
     class Reader : public boost::noncopyable
     {
     private:
       Orthanc::MemoryObjectCache::Reader reader_;
+      std::auto_ptr<boost::mutex::scoped_lock>  lock_;
+      Item*                         item_;
 
     public:
       Reader(ParsedDicomFileCache& cache,
-             const std::string& sopInstanceUid) :
-        reader_(cache.cache_, sopInstanceUid)
-      {
-      }
+             const std::string& path);
 
       bool IsValid() const
       {
-        return reader_.IsValid();
+        return item_ != NULL;
       }
 
-      const Orthanc::ParsedDicomFile& GetDicom() const;
+      bool HasPixelData() const;
+
+      boost::shared_ptr<Orthanc::ParsedDicomFile> GetDicom() const;
+
+      size_t GetFileSize() const;
     };
   };
 }