Mercurial > hg > orthanc
changeset 3015:abe49ca61cd5
On C-FIND, avoid accessing the storage area whenever possible
author | Sebastien Jodogne <s.jodogne@gmail.com> |
---|---|
date | Fri, 14 Dec 2018 12:10:03 +0100 |
parents | b9f0b0c0b36f |
children | 777762336381 2cbafb5d5a62 |
files | Core/DicomFormat/DicomMap.cpp Core/DicomFormat/DicomMap.h NEWS OrthancServer/OrthancFindRequestHandler.cpp OrthancServer/ServerContext.cpp OrthancServer/ServerContext.h Resources/Configuration.json UnitTestsSources/DicomMapTests.cpp |
diffstat | 8 files changed, 254 insertions(+), 151 deletions(-) [+] |
line wrap: on
line diff
--- a/Core/DicomFormat/DicomMap.cpp Thu Dec 13 17:58:27 2018 +0100 +++ b/Core/DicomFormat/DicomMap.cpp Fri Dec 14 12:10:03 2018 +0100 @@ -1069,6 +1069,25 @@ } + bool DicomMap::HasOnlyMainDicomTags() const + { + // TODO - Speed up possible by making this std::set a global variable + + std::set<DicomTag> mainDicomTags; + GetMainDicomTags(mainDicomTags); + + for (Map::const_iterator it = map_.begin(); it != map_.end(); ++it) + { + if (mainDicomTags.find(it->first) == mainDicomTags.end()) + { + return false; + } + } + + return true; + } + + void DicomMap::Serialize(Json::Value& target) const { target = Json::objectValue;
--- a/Core/DicomFormat/DicomMap.h Thu Dec 13 17:58:27 2018 +0100 +++ b/Core/DicomFormat/DicomMap.h Fri Dec 14 12:10:03 2018 +0100 @@ -224,7 +224,9 @@ void Merge(const DicomMap& other); - void ExtractMainDicomTags(const DicomMap& other); + void ExtractMainDicomTags(const DicomMap& other); + + bool HasOnlyMainDicomTags() const; void Serialize(Json::Value& target) const;
--- a/NEWS Thu Dec 13 17:58:27 2018 +0100 +++ b/NEWS Fri Dec 14 12:10:03 2018 +0100 @@ -5,13 +5,15 @@ General ------- -* Optimization: On finds, do not read JSON (disk) if main DICOM tags (DB) are sufficient +* Optimization: On C-FIND, avoid accessing the storage area whenever possible +* New configuration option: + - "StorageAccessOnFind" to rule the access to the storage area during C-FIND Maintenance ----------- -* "/tools/create-dicom" is more tolerant to invalid specific character set - +* Removal of the "AllowFindSopClassesInStudy" old configuration option +* "/tools/create-dicom" is more tolerant wrt. invalid specific character set Version 1.5.0 (2018-12-10)
--- a/OrthancServer/OrthancFindRequestHandler.cpp Thu Dec 13 17:58:27 2018 +0100 +++ b/OrthancServer/OrthancFindRequestHandler.cpp Fri Dec 14 12:10:03 2018 +0100 @@ -89,82 +89,6 @@ } - static void ExtractTagFromMainDicomTags(std::set<std::string>& target, - ServerIndex& index, - const DicomTag& tag, - const std::list<std::string>& resources, - ResourceType level) - { - for (std::list<std::string>::const_iterator - it = resources.begin(); it != resources.end(); ++it) - { - DicomMap tags; - if (index.GetMainDicomTags(tags, *it, level, level) && - tags.HasTag(tag)) - { - target.insert(tags.GetValue(tag).GetContent()); - } - } - } - - - static bool ExtractMetadata(std::set<std::string>& target, - ServerIndex& index, - MetadataType metadata, - const std::list<std::string>& resources) - { - for (std::list<std::string>::const_iterator - it = resources.begin(); it != resources.end(); ++it) - { - std::string value; - if (index.LookupMetadata(value, *it, metadata)) - { - target.insert(value); - } - else - { - // This metadata is unavailable for some resource, give up - return false; - } - } - - return true; - } - - - static void ExtractTagFromInstancesOnDisk(std::set<std::string>& target, - ServerContext& context, - const DicomTag& tag, - const std::list<std::string>& instances) - { - // WARNING: This function is slow, as it reads the JSON file - // summarizing each instance of interest from the hard drive. - - std::string formatted = tag.Format(); - - for (std::list<std::string>::const_iterator - it = instances.begin(); it != instances.end(); ++it) - { - Json::Value dicom; - context.ReadDicomAsJson(dicom, *it); - - if (dicom.isMember(formatted)) - { - const Json::Value& source = dicom[formatted]; - - if (source.type() == Json::objectValue && - source.isMember("Type") && - source.isMember("Value") && - source["Type"].asString() == "String" && - source["Value"].type() == Json::stringValue) - { - target.insert(source["Value"].asString()); - } - } - } - } - - static void ComputePatientCounters(DicomMap& result, ServerIndex& index, const std::string& patient, @@ -230,7 +154,24 @@ if (query.HasTag(DICOM_TAG_MODALITIES_IN_STUDY)) { std::set<std::string> values; - ExtractTagFromMainDicomTags(values, index, DICOM_TAG_MODALITY, series, ResourceType_Series); + + for (std::list<std::string>::const_iterator + it = series.begin(); it != series.end(); ++it) + { + DicomMap tags; + if (index.GetMainDicomTags(tags, *it, ResourceType_Series, ResourceType_Series)) + { + const DicomValue* value = tags.TestAndGetValue(DICOM_TAG_MODALITY); + + if (value != NULL && + !value->IsNull() && + !value->IsBinary()) + { + values.insert(value->GetContent()); + } + } + } + StoreSetOfStrings(result, DICOM_TAG_MODALITIES_IN_STUDY, values); } @@ -253,27 +194,17 @@ { std::set<std::string> values; - if (ExtractMetadata(values, index, MetadataType_Instance_SopClassUid, instances)) - { - // The metadata "SopClassUid" is available for each of these instances - StoreSetOfStrings(result, DICOM_TAG_SOP_CLASSES_IN_STUDY, values); - } - else + for (std::list<std::string>::const_iterator + it = instances.begin(); it != instances.end(); ++it) { - OrthancConfiguration::ReaderLock lock; - - if (lock.GetConfiguration().GetBooleanParameter("AllowFindSopClassesInStudy", false)) + std::string value; + if (context.LookupOrReconstructMetadata(value, *it, MetadataType_Instance_SopClassUid)) { - ExtractTagFromInstancesOnDisk(values, context, DICOM_TAG_SOP_CLASS_UID, instances); - StoreSetOfStrings(result, DICOM_TAG_SOP_CLASSES_IN_STUDY, values); - } - else - { - result.SetValue(DICOM_TAG_SOP_CLASSES_IN_STUDY, "", false); - LOG(WARNING) << "The handling of \"SOP Classes in Study\" (0008,0062) " - << "in C-FIND requests is disabled"; + values.insert(value); } } + + StoreSetOfStrings(result, DICOM_TAG_SOP_CLASSES_IN_STUDY, values); } } @@ -366,11 +297,22 @@ static void AddAnswer(DicomFindAnswers& answers, const DicomMap& mainDicomTags, - const Json::Value* dicomAsJson, // only used for sequences + const Json::Value* dicomAsJson, const DicomArray& query, const std::list<DicomTag>& sequencesToReturn, const DicomMap* counters) { + DicomMap match; + + if (dicomAsJson != NULL) + { + match.FromDicomAsJson(*dicomAsJson); + } + else + { + match.Assign(mainDicomTags); + } + DicomMap result; for (size_t i = 0; i < query.GetSize(); i++) @@ -384,26 +326,22 @@ { // Do not include the encoding, this is handled by class ParsedDicomFile } - else if (dicomAsJson != NULL) + else { - std::string tag = query.GetElement(i).GetTag().Format(); - std::string value; - if (dicomAsJson->isMember(tag)) + const DicomTag& tag = query.GetElement(i).GetTag(); + const DicomValue* value = match.TestAndGetValue(tag); + + if (value != NULL && + !value->IsNull() && + !value->IsBinary()) { - value = dicomAsJson->get(tag, Json::arrayValue).get("Value", "").asString(); - result.SetValue(query.GetElement(i).GetTag(), value, false); + result.SetValue(tag, value->GetContent(), false); } else { - result.SetValue(query.GetElement(i).GetTag(), "", false); + result.SetValue(tag, "", false); } } - else - { - // Best-effort - // TODO - throw OrthancException(ErrorCode_NotImplemented); - } } if (counters != NULL) @@ -420,11 +358,15 @@ { LOG(WARNING) << "The C-FIND request does not return any DICOM tag"; } - else if (sequencesToReturn.empty() || - dicomAsJson == NULL) + else if (sequencesToReturn.empty()) { answers.Add(result); } + else if (dicomAsJson == NULL) + { + LOG(WARNING) << "C-FIND query requesting a sequence, but reading JSON from disk is disabled"; + answers.Add(result); + } else { ParsedDicomFile dicom(result); @@ -536,48 +478,50 @@ DicomFindAnswers& answers_; ServerContext& context_; ResourceType level_; - const DicomMap& filteredInput_; + const DicomMap& query_; + DicomArray queryAsArray_; const std::list<DicomTag>& sequencesToReturn_; - DicomArray query_; public: LookupVisitor(DicomFindAnswers& answers, ServerContext& context, ResourceType level, - const DicomMap& filteredInput, + const DicomMap& query, const std::list<DicomTag>& sequencesToReturn) : answers_(answers), context_(context), level_(level), - filteredInput_(filteredInput), - sequencesToReturn_(sequencesToReturn), - query_(filteredInput) + query_(query), + queryAsArray_(query), + sequencesToReturn_(sequencesToReturn) { 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; + DicomMap withoutSpecialTags; + withoutSpecialTags.Assign(query_); + + // Check out "ComputeCounters()" + withoutSpecialTags.Remove(DICOM_TAG_MODALITIES_IN_STUDY); + withoutSpecialTags.Remove(DICOM_TAG_NUMBER_OF_PATIENT_RELATED_INSTANCES); + withoutSpecialTags.Remove(DICOM_TAG_NUMBER_OF_PATIENT_RELATED_SERIES); + withoutSpecialTags.Remove(DICOM_TAG_NUMBER_OF_PATIENT_RELATED_STUDIES); + withoutSpecialTags.Remove(DICOM_TAG_NUMBER_OF_SERIES_RELATED_INSTANCES); + withoutSpecialTags.Remove(DICOM_TAG_NUMBER_OF_STUDY_RELATED_INSTANCES); + withoutSpecialTags.Remove(DICOM_TAG_NUMBER_OF_STUDY_RELATED_SERIES); + withoutSpecialTags.Remove(DICOM_TAG_SOP_CLASSES_IN_STUDY); + + // Check out "AddAnswer()" + withoutSpecialTags.Remove(DICOM_TAG_SPECIFIC_CHARACTER_SET); + withoutSpecialTags.Remove(DICOM_TAG_QUERY_RETRIEVE_LEVEL); - { - // New configuration option in 1.5.1 - OrthancConfiguration::ReaderLock lock; - findFromDatabase = lock.GetConfiguration().GetUnsignedIntegerParameter("FindFromDatabase", false); - } - - return !sequencesToReturn_.empty(); -#endif + return (!sequencesToReturn_.empty() || + !withoutSpecialTags.HasOnlyMainDicomTags()); } virtual void MarkAsComplete() @@ -590,8 +534,10 @@ const DicomMap& mainDicomTags, const Json::Value* dicomAsJson) { - std::auto_ptr<DicomMap> counters(ComputeCounters(context_, instanceId, level_, filteredInput_)); - AddAnswer(answers_, mainDicomTags, dicomAsJson, query_, sequencesToReturn_, counters.get()); + std::auto_ptr<DicomMap> counters(ComputeCounters(context_, instanceId, level_, query_)); + + AddAnswer(answers_, mainDicomTags, dicomAsJson, + queryAsArray_, sequencesToReturn_, counters.get()); } };
--- a/OrthancServer/ServerContext.cpp Thu Dec 13 17:58:27 2018 +0100 +++ b/OrthancServer/ServerContext.cpp Fri Dec 14 12:10:03 2018 +0100 @@ -778,6 +778,35 @@ size_t since, size_t limit) { + LookupMode mode; + + { + // New configuration option in 1.5.1 + OrthancConfiguration::ReaderLock lock; + + std::string value = lock.GetConfiguration().GetStringParameter("StorageAccessOnFind", "Always"); + + if (value == "Always") + { + mode = LookupMode_DiskOnLookupAndAnswer; + } + else if (value == "Never") + { + mode = LookupMode_DatabaseOnly; + } + else if (value == "Answers") + { + mode = LookupMode_DiskOnAnswer; + } + else + { + throw OrthancException(ErrorCode_ParameterOutOfRange, + "Configuration option \"StorageAccessOnFind\" " + "should be \"Always\", \"Never\" or \"Answers\": " + value); + } + } + + std::vector<std::string> resources, instances; GetIndex().FindCandidates(resources, instances, lookup); @@ -788,6 +817,8 @@ size_t countResults = 0; size_t skipped = 0; bool complete = true; + + const bool isDicomAsJsonNeeded = visitor.IsDicomAsJsonNeeded(); for (size_t i = 0; i < instances.size(); i++) { @@ -799,11 +830,20 @@ bool hasOnlyMainDicomTags; DicomMap dicom; - if (lookup.HasOnlyMainDicomTags() && - GetIndex().GetAllMainDicomTags(dicom, instances[i])) + if (mode == LookupMode_DatabaseOnly || + mode == LookupMode_DiskOnAnswer || + lookup.HasOnlyMainDicomTags()) { // Case (1): The main DICOM tags, as stored in the database, // are sufficient to look for match + + if (!GetIndex().GetAllMainDicomTags(dicom, instances[i])) + { + // The instance has been removed during the execution of the + // lookup, ignore it + continue; + } + hasOnlyMainDicomTags = true; } else @@ -834,8 +874,10 @@ } else { - if (dicomAsJson.get() == NULL && - visitor.IsDicomAsJsonNeeded()) + if ((mode == LookupMode_DiskOnLookupAndAnswer || + mode == LookupMode_DiskOnAnswer) && + dicomAsJson.get() == NULL && + isDicomAsJsonNeeded) { dicomAsJson.reset(new Json::Value); ReadDicomAsJson(*dicomAsJson, instances[i]); @@ -870,6 +912,77 @@ } + bool ServerContext::LookupOrReconstructMetadata(std::string& target, + const std::string& publicId, + MetadataType metadata) + { + // This is a backwards-compatibility function, that can + // reconstruct metadata that were not generated by an older + // release of Orthanc + + if (metadata == MetadataType_Instance_SopClassUid || + metadata == MetadataType_Instance_TransferSyntax) + { + if (index_.LookupMetadata(target, publicId, metadata)) + { + return true; + } + else + { + // These metadata are mandatory in DICOM instances, and were + // introduced in Orthanc 1.2.0. The fact that + // "LookupMetadata()" has failed indicates that this database + // comes from an older release of Orthanc. + + DicomTag tag(0, 0); + + switch (metadata) + { + case MetadataType_Instance_SopClassUid: + tag = DICOM_TAG_SOP_CLASS_UID; + break; + + case MetadataType_Instance_TransferSyntax: + tag = DICOM_TAG_TRANSFER_SYNTAX_UID; + break; + + default: + throw OrthancException(ErrorCode_InternalError); + } + + Json::Value dicomAsJson; + ReadDicomAsJson(dicomAsJson, publicId); + + DicomMap tags; + tags.FromDicomAsJson(dicomAsJson); + + const DicomValue* value = tags.TestAndGetValue(tag); + + if (value != NULL && + !value->IsNull() && + !value->IsBinary()) + { + target = value->GetContent(); + + // Store for reuse + index_.SetMetadata(publicId, metadata, target); + return true; + } + else + { + // Should never happen + return false; + } + } + } + else + { + // No backward + return index_.LookupMetadata(target, publicId, metadata); + } + } + + void ServerContext::AddChildInstances(SetOfInstancesJob& job, const std::string& publicId) {
--- a/OrthancServer/ServerContext.h Thu Dec 13 17:58:27 2018 +0100 +++ b/OrthancServer/ServerContext.h Fri Dec 14 12:10:03 2018 +0100 @@ -64,6 +64,14 @@ class ServerContext : private JobsRegistry::IObserver { private: + enum LookupMode + { + LookupMode_DatabaseOnly, + LookupMode_DiskOnAnswer, + LookupMode_DiskOnLookupAndAnswer + }; + + class LuaServerListener : public IServerListener { private: @@ -340,6 +348,10 @@ size_t since, size_t limit); + bool LookupOrReconstructMetadata(std::string& target, + const std::string& publicId, + MetadataType type); + /** * Management of the plugins
--- a/Resources/Configuration.json Thu Dec 13 17:58:27 2018 +0100 +++ b/Resources/Configuration.json Fri Dec 14 12:10:03 2018 +0100 @@ -384,14 +384,6 @@ } **/ - // If set to "true", Orthanc will still handle "SOP Classes in - // Study" (0008,0062) in C-FIND requests, even if the "SOP Class - // UID" metadata is not available in the database (which is the case - // if the DB was previously used by Orthanc <= 1.1.0). This option - // is turned off by default, as it requires intensive accesses to - // the hard drive. - "AllowFindSopClassesInStudy" : false, - // If set to "false", Orthanc will not load its default dictionary // of private tags. This might be necessary if you cannot import a // DICOM file encoded using the Implicit VR Endian transfer syntax, @@ -447,5 +439,17 @@ // The least recently used archives get deleted as new archives are // generated. This option was introduced in Orthanc 1.5.0, and has // no effect on the synchronous generation of archives. - "MediaArchiveSize" : 1 + "MediaArchiveSize" : 1, + + // Performance setting to specify how Orthanc accesses the storage + // area during C-FIND. Three modes are available: (1) "Always" + // allows Orthanc to read the storage area as soon as it needs an + // information that is not present in its database (slowest mode), + // (2) "Never" prevents Orthanc from accessing the storage area, and + // makes it uses exclusively its database (fastest mode), and (3) + // "Answers" allows Orthanc to read the storage area to generate its + // answers, but not to filter the DICOM resources (balance between + // the two modes). By default, the mode is "Always", which + // corresponds to the behavior of Orthanc <= 1.5.0. + "StorageAccessOnFind" : "Always" }
--- a/UnitTestsSources/DicomMapTests.cpp Thu Dec 13 17:58:27 2018 +0100 +++ b/UnitTestsSources/DicomMapTests.cpp Fri Dec 14 12:10:03 2018 +0100 @@ -511,6 +511,7 @@ { DicomMap b; b.SetValue(DICOM_TAG_PATIENT_NAME, "E", false); + ASSERT_TRUE(b.HasOnlyMainDicomTags()); { DicomMap a; @@ -519,6 +520,7 @@ a.SetValue(DICOM_TAG_SERIES_DESCRIPTION, "C", false); a.SetValue(DICOM_TAG_NUMBER_OF_FRAMES, "D", false); a.SetValue(DICOM_TAG_SLICE_THICKNESS, "F", false); + ASSERT_FALSE(a.HasOnlyMainDicomTags()); b.ExtractMainDicomTags(a); } @@ -528,6 +530,7 @@ ASSERT_EQ("C", b.GetValue(DICOM_TAG_SERIES_DESCRIPTION).GetContent()); ASSERT_EQ("D", b.GetValue(DICOM_TAG_NUMBER_OF_FRAMES).GetContent()); ASSERT_FALSE(b.HasTag(DICOM_TAG_SLICE_THICKNESS)); + ASSERT_TRUE(b.HasOnlyMainDicomTags()); b.SetValue(DICOM_TAG_PATIENT_NAME, "G", false); @@ -535,6 +538,7 @@ DicomMap a; a.SetValue(DICOM_TAG_PATIENT_NAME, "A", false); a.SetValue(DICOM_TAG_SLICE_THICKNESS, "F", false); + ASSERT_FALSE(a.HasOnlyMainDicomTags()); b.Merge(a); } @@ -544,4 +548,5 @@ ASSERT_EQ("C", b.GetValue(DICOM_TAG_SERIES_DESCRIPTION).GetContent()); ASSERT_EQ("D", b.GetValue(DICOM_TAG_NUMBER_OF_FRAMES).GetContent()); ASSERT_EQ("F", b.GetValue(DICOM_TAG_SLICE_THICKNESS).GetContent()); + ASSERT_FALSE(b.HasOnlyMainDicomTags()); }