# HG changeset patch # User Sebastien Jodogne # Date 1544720046 -3600 # Node ID af1530b452907844f077c59998b2b69f40287b27 # Parent 9f859a18cbc2ca9a8ef935d3764116079a2ff86a Optimization: On finds, do not read JSON (disk) if main DICOM tags (DB) are sufficient diff -r 9f859a18cbc2 -r af1530b45290 NEWS --- a/NEWS Thu Dec 13 17:16:32 2018 +0100 +++ b/NEWS Thu Dec 13 17:54:06 2018 +0100 @@ -2,6 +2,11 @@ =============================== +General +------- + +* Optimization: On finds, do not read JSON (disk) if main DICOM tags (DB) are sufficient + Maintenance ----------- diff -r 9f859a18cbc2 -r af1530b45290 OrthancServer/OrthancFindRequestHandler.cpp --- a/OrthancServer/OrthancFindRequestHandler.cpp Thu Dec 13 17:16:32 2018 +0100 +++ b/OrthancServer/OrthancFindRequestHandler.cpp Thu Dec 13 17:54:06 2018 +0100 @@ -365,7 +365,8 @@ static void AddAnswer(DicomFindAnswers& answers, - const Json::Value& resource, + const DicomMap& mainDicomTags, + const Json::Value* dicomAsJson, // only used for sequences const DicomArray& query, const std::list& sequencesToReturn, const DicomMap* counters) @@ -383,13 +384,13 @@ { // Do not include the encoding, this is handled by class ParsedDicomFile } - else + else if (dicomAsJson != NULL) { std::string tag = query.GetElement(i).GetTag().Format(); std::string value; - if (resource.isMember(tag)) + if (dicomAsJson->isMember(tag)) { - value = resource.get(tag, Json::arrayValue).get("Value", "").asString(); + value = dicomAsJson->get(tag, Json::arrayValue).get("Value", "").asString(); result.SetValue(query.GetElement(i).GetTag(), value, false); } else @@ -397,6 +398,12 @@ result.SetValue(query.GetElement(i).GetTag(), "", false); } } + else + { + // Best-effort + // TODO + throw OrthancException(ErrorCode_NotImplemented); + } } if (counters != NULL) @@ -413,7 +420,8 @@ { LOG(WARNING) << "The C-FIND request does not return any DICOM tag"; } - else if (sequencesToReturn.empty()) + else if (sequencesToReturn.empty() || + dicomAsJson == NULL) { answers.Add(result); } @@ -424,7 +432,8 @@ for (std::list::const_iterator tag = sequencesToReturn.begin(); tag != sequencesToReturn.end(); ++tag) { - const Json::Value& source = resource[tag->Format()]; + assert(dicomAsJson != NULL); + const Json::Value& source = (*dicomAsJson) [tag->Format()]; if (source.type() == Json::objectValue && source.isMember("Type") && @@ -546,6 +555,30 @@ { answers_.SetComplete(false); } + + virtual bool IsDicomAsJsonNeeded() const + { +#if 1 + return true; + +#else + // TODO + + // Ask the "DICOM-as-JSON" attachment only if sequences are to + // be returned OR if "query_" contains non-main DICOM tags! + + // TODO - configuration option + bool findFromDatabase; + + { + // New configuration option in 1.5.1 + OrthancConfiguration::ReaderLock lock; + findFromDatabase = lock.GetConfiguration().GetUnsignedIntegerParameter("FindFromDatabase", false); + } + + return !sequencesToReturn_.empty(); +#endif + } virtual void MarkAsComplete() { @@ -554,10 +587,11 @@ virtual void Visit(const std::string& publicId, const std::string& instanceId, - const Json::Value& dicom) + const DicomMap& mainDicomTags, + const Json::Value* dicomAsJson) { std::auto_ptr counters(ComputeCounters(context_, instanceId, level_, filteredInput_)); - AddAnswer(answers_, dicom, query_, sequencesToReturn_, counters.get()); + AddAnswer(answers_, mainDicomTags, dicomAsJson, query_, sequencesToReturn_, counters.get()); } }; @@ -690,6 +724,7 @@ size_t limit = (level == ResourceType_Instance) ? maxInstances_ : maxResults_; + LookupVisitor visitor(answers, context_, level, *filteredInput, sequencesToReturn); context_.Apply(visitor, lookup, 0 /* "since" is not relevant to C-FIND */, limit); } diff -r 9f859a18cbc2 -r af1530b45290 OrthancServer/OrthancRestApi/OrthancRestResources.cpp --- a/OrthancServer/OrthancRestApi/OrthancRestResources.cpp Thu Dec 13 17:16:32 2018 +0100 +++ b/OrthancServer/OrthancRestApi/OrthancRestResources.cpp Thu Dec 13 17:54:06 2018 +0100 @@ -1283,14 +1283,20 @@ { } + virtual bool IsDicomAsJsonNeeded() const + { + return false; // (*) + } + virtual void MarkAsComplete() { isComplete_ = true; // Unused information as of Orthanc 1.5.0 } virtual void Visit(const std::string& publicId, - const std::string& instanceId /* unused */, - const Json::Value& dicom /* unused */) + const std::string& instanceId /* unused */, + const DicomMap& mainDicomTags /* unused */, + const Json::Value* dicomAsJson /* unused (*) */) { resources_.push_back(publicId); } diff -r 9f859a18cbc2 -r af1530b45290 OrthancServer/Search/LookupResource.cpp --- a/OrthancServer/Search/LookupResource.cpp Thu Dec 13 17:16:32 2018 +0100 +++ b/OrthancServer/Search/LookupResource.cpp Thu Dec 13 17:54:06 2018 +0100 @@ -42,6 +42,19 @@ namespace Orthanc { + static bool DoesDicomMapMatch(const DicomMap& dicom, + const DicomTag& tag, + const IFindConstraint& constraint) + { + const DicomValue* value = dicom.TestAndGetValue(tag); + + return (value != NULL && + !value->IsNull() && + !value->IsBinary() && + constraint.Match(value->GetContent())); + } + + LookupResource::Level::Level(ResourceType level) : level_(level) { const DicomTag* tags = NULL; @@ -119,6 +132,34 @@ } + bool LookupResource::Level::IsMatch(const DicomMap& dicom) const + { + for (Constraints::const_iterator it = identifiersConstraints_.begin(); + it != identifiersConstraints_.end(); ++it) + { + assert(it->second != NULL); + + if (!DoesDicomMapMatch(dicom, it->first, *it->second)) + { + return false; + } + } + + for (Constraints::const_iterator it = mainTagsConstraints_.begin(); + it != mainTagsConstraints_.end(); ++it) + { + assert(it->second != NULL); + + if (!DoesDicomMapMatch(dicom, it->first, *it->second)) + { + return false; + } + } + + return true; + } + + LookupResource::LookupResource(ResourceType level) : level_(level) { switch (level) @@ -283,22 +324,22 @@ - bool LookupResource::IsMatch(const Json::Value& dicomAsJson) const + bool LookupResource::IsMatch(const DicomMap& dicom) const { + for (Levels::const_iterator it = levels_.begin(); it != levels_.end(); ++it) + { + if (!it->second->IsMatch(dicom)) + { + return false; + } + } + for (Constraints::const_iterator it = unoptimizedConstraints_.begin(); it != unoptimizedConstraints_.end(); ++it) { - std::string tag = it->first.Format(); - if (dicomAsJson.isMember(tag) && - dicomAsJson[tag]["Type"] == "String") - { - std::string value = dicomAsJson[tag]["Value"].asString(); - if (!it->second->Match(value)) - { - return false; - } - } - else + assert(it->second != NULL); + + if (!DoesDicomMapMatch(dicom, it->first, *it->second)) { return false; } diff -r 9f859a18cbc2 -r af1530b45290 OrthancServer/Search/LookupResource.h --- a/OrthancServer/Search/LookupResource.h Thu Dec 13 17:16:32 2018 +0100 +++ b/OrthancServer/Search/LookupResource.h Thu Dec 13 17:54:06 2018 +0100 @@ -64,6 +64,8 @@ void Apply(SetOfResources& candidates, IDatabaseWrapper& database) const; + + bool IsMatch(const DicomMap& dicom) const; }; typedef std::map Levels; @@ -89,11 +91,14 @@ { } + virtual bool IsDicomAsJsonNeeded() const = 0; + virtual void MarkAsComplete() = 0; virtual void Visit(const std::string& publicId, const std::string& instanceId, - const Json::Value& dicom) = 0; + const DicomMap& mainDicomTags, + const Json::Value* dicomAsJson) = 0; }; LookupResource(ResourceType level); @@ -117,6 +122,11 @@ void FindCandidates(std::list& result, IDatabaseWrapper& database) const; - bool IsMatch(const Json::Value& dicomAsJson) const; + bool HasOnlyMainDicomTags() const + { + return unoptimizedConstraints_.empty(); + } + + bool IsMatch(const DicomMap& dicom) const; }; } diff -r 9f859a18cbc2 -r af1530b45290 OrthancServer/ServerContext.cpp --- a/OrthancServer/ServerContext.cpp Thu Dec 13 17:16:32 2018 +0100 +++ b/OrthancServer/ServerContext.cpp Thu Dec 13 17:54:06 2018 +0100 @@ -788,13 +788,36 @@ size_t countResults = 0; size_t skipped = 0; bool complete = true; - + for (size_t i = 0; i < instances.size(); i++) { - // TODO - Don't read the full JSON from the disk if only "main - // DICOM tags" are to be returned - Json::Value dicom; - ReadDicomAsJson(dicom, instances[i]); + // Optimization in Orthanc 1.5.1 - Don't read the full JSON from + // the disk if only "main DICOM tags" are to be returned + + std::auto_ptr dicomAsJson; + + bool hasOnlyMainDicomTags; + DicomMap dicom; + + if (lookup.HasOnlyMainDicomTags() && + GetIndex().GetAllMainDicomTags(dicom, instances[i])) + { + // Case (1): The main DICOM tags, as stored in the database, + // are sufficient to look for match + hasOnlyMainDicomTags = true; + } + else + { + // Case (2): Need to read the "DICOM-as-JSON" attachment from + // the storage area + dicomAsJson.reset(new Json::Value); + ReadDicomAsJson(*dicomAsJson, instances[i]); + + dicom.FromDicomAsJson(*dicomAsJson); + + // This map contains the entire JSON, i.e. more than the main DICOM tags + hasOnlyMainDicomTags = false; + } if (lookup.IsMatch(dicom)) { @@ -811,7 +834,28 @@ } else { - visitor.Visit(resources[i], instances[i], dicom); + if (dicomAsJson.get() == NULL && + visitor.IsDicomAsJsonNeeded()) + { + dicomAsJson.reset(new Json::Value); + ReadDicomAsJson(*dicomAsJson, instances[i]); + } + + if (hasOnlyMainDicomTags) + { + // This is Case (1): The variable "dicom" only contains the main DICOM tags + visitor.Visit(resources[i], instances[i], dicom, dicomAsJson.get()); + } + else + { + // Remove the non-main DICOM tags from "dicom" if Case (2) + // was used, for consistency with Case (1) + + DicomMap mainDicomTags; + mainDicomTags.ExtractMainDicomTags(dicom); + visitor.Visit(resources[i], instances[i], mainDicomTags, dicomAsJson.get()); + } + countResults ++; } }