From 9a73079c33133e50dc4fd4f77cd5207ab95bc2c1 Mon Sep 17 00:00:00 2001 From: Max Bureck Date: Tue, 8 Oct 2024 15:29:42 +0200 Subject: [PATCH] Improving performance, using caching when testing for primitives (#6252) (#6253) * Improving performance, using caching when testing for primitives (#6252) Caching primitive type names for faster lookup if a type is primitive. * Credit for #6253 --------- Co-authored-by: James Agnew --- .../6252-improve-validator-performance.yaml | 6 ++ .../VersionSpecificWorkerContextWrapper.java | 27 ++++++-- ...rsionSpecificWorkerContextWrapperTest.java | 65 +++++++++++++++++++ pom.xml | 4 ++ 4 files changed, 97 insertions(+), 5 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_6_0/6252-improve-validator-performance.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_6_0/6252-improve-validator-performance.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_6_0/6252-improve-validator-performance.yaml new file mode 100644 index 00000000000..a27d92ec4b7 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_6_0/6252-improve-validator-performance.yaml @@ -0,0 +1,6 @@ +--- +type: perf +issue: 6253 +title: "A cache has been added to the validation services layer which results + in improved validation performance. Thanks to Max Bureck for the + contribution!" diff --git a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapper.java b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapper.java index 64d4f75519a..f0f3f41e7f9 100644 --- a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapper.java +++ b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapper.java @@ -55,11 +55,15 @@ import org.slf4j.LoggerFactory; import java.io.FileNotFoundException; import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Objects; import java.util.Set; +import static java.util.stream.Collectors.collectingAndThen; +import static java.util.stream.Collectors.toSet; import static org.apache.commons.lang3.StringUtils.isBlank; import static org.apache.commons.lang3.StringUtils.isNotBlank; @@ -69,6 +73,7 @@ public class VersionSpecificWorkerContextWrapper extends I18nBase implements IWo private final VersionCanonicalizer myVersionCanonicalizer; private final LoadingCache myFetchResourceCache; private volatile List myAllStructures; + private volatile Set myAllPrimitiveTypes; private Parameters myExpansionProfile; public VersionSpecificWorkerContextWrapper( @@ -617,11 +622,23 @@ public class VersionSpecificWorkerContextWrapper extends I18nBase implements IWo @Override public boolean isPrimitiveType(String theType) { - List allStructures = new ArrayList<>(allStructures()); - return allStructures.stream() - .filter(structureDefinition -> - structureDefinition.getKind() == StructureDefinition.StructureDefinitionKind.PRIMITIVETYPE) - .anyMatch(structureDefinition -> theType.equals(structureDefinition.getName())); + return allPrimitiveTypes().contains(theType); + } + + private Set allPrimitiveTypes() { + Set retVal = myAllPrimitiveTypes; + if (retVal == null) { + // Collector may be changed to Collectors.toUnmodifiableSet() when switching to Android API level >= 33 + retVal = allStructures().stream() + .filter(structureDefinition -> + structureDefinition.getKind() == StructureDefinition.StructureDefinitionKind.PRIMITIVETYPE) + .map(StructureDefinition::getName) + .filter(Objects::nonNull) + .collect(collectingAndThen(toSet(), Collections::unmodifiableSet)); + myAllPrimitiveTypes = retVal; + } + + return retVal; } @Override diff --git a/hapi-fhir-validation/src/test/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapperTest.java b/hapi-fhir-validation/src/test/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapperTest.java index ceccb4b5560..57aae8d96f9 100644 --- a/hapi-fhir-validation/src/test/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapperTest.java +++ b/hapi-fhir-validation/src/test/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapperTest.java @@ -7,7 +7,10 @@ import ca.uhn.fhir.context.support.ValidationSupportContext; import ca.uhn.fhir.fhirpath.BaseValidationTestWithInlineMocks; import ca.uhn.fhir.i18n.HapiLocalizer; import ca.uhn.hapi.converters.canonical.VersionCanonicalizer; + import org.hl7.fhir.r5.model.Resource; +import org.hl7.fhir.r5.model.StructureDefinition; +import org.hl7.fhir.r5.model.StructureDefinition.StructureDefinitionKind; import org.hl7.fhir.r5.model.ValueSet; import org.hl7.fhir.utilities.validation.ValidationOptions; import org.junit.jupiter.api.Test; @@ -22,6 +25,8 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.mockito.Mockito.withSettings; +import java.util.List; + public class VersionSpecificWorkerContextWrapperTest extends BaseValidationTestWithInlineMocks { final byte[] EXPECTED_BINARY_CONTENT_1 = "dummyBinaryContent1".getBytes(); @@ -96,6 +101,66 @@ public class VersionSpecificWorkerContextWrapperTest extends BaseValidationTestW verify(validationSupport, times(1)).validateCode(any(), any(), eq("http://codesystems.com/system"), eq("code0"), any(), any()); } + @Test + public void isPrimitive_primitive() { + // setup + IValidationSupport validationSupport = mockValidationSupport(); + ValidationSupportContext mockContext = mockValidationSupportContext(validationSupport); + VersionCanonicalizer versionCanonicalizer = new VersionCanonicalizer(FhirContext.forR5Cached()); + VersionSpecificWorkerContextWrapper wrapper = new VersionSpecificWorkerContextWrapper(mockContext, versionCanonicalizer); + + List structDefs = createStructureDefinitions(); + + when(mockContext.getRootValidationSupport().fetchAllStructureDefinitions()).thenReturn(structDefs); + assertThat(wrapper.isPrimitiveType("boolean")).isTrue(); + + // try again to check if lookup after cache is built is working + assertThat(wrapper.isPrimitiveType("string")).isTrue(); + } + + @Test + public void isPrimitive_not_primitive() { + // setup + IValidationSupport validationSupport = mockValidationSupport(); + ValidationSupportContext mockContext = mockValidationSupportContext(validationSupport); + VersionCanonicalizer versionCanonicalizer = new VersionCanonicalizer(FhirContext.forR5Cached()); + VersionSpecificWorkerContextWrapper wrapper = new VersionSpecificWorkerContextWrapper(mockContext, versionCanonicalizer); + + List structDefs = createStructureDefinitions(); + + when(mockContext.getRootValidationSupport().fetchAllStructureDefinitions()).thenReturn(structDefs); + assertThat(wrapper.isPrimitiveType("Person")).isFalse(); + + // try again to check if lookup after cache is built is working + assertThat(wrapper.isPrimitiveType("Organization")).isFalse(); + + // Assert that unknown types are not regarded as primitive + assertThat(wrapper.isPrimitiveType("Unknown")).isFalse(); + } + + private List createStructureDefinitions() { + StructureDefinition stringType = createPrimitive("string"); + StructureDefinition boolType = createPrimitive("boolean"); + StructureDefinition personType = createComplex("Person"); + StructureDefinition orgType = createComplex("Organization"); + + return List.of(personType, boolType, orgType, stringType); + } + + private StructureDefinition createComplex(String name){ + return createStructureDefinition(name).setKind(StructureDefinitionKind.COMPLEXTYPE); + } + + private StructureDefinition createPrimitive(String name){ + return createStructureDefinition(name).setKind(StructureDefinitionKind.PRIMITIVETYPE); + } + + private StructureDefinition createStructureDefinition(String name) { + StructureDefinition sd = new StructureDefinition(); + sd.setUrl("http://hl7.org/fhir/StructureDefinition/"+name).setName(name); + return sd; + } + private IValidationSupport mockValidationSupportWithTwoBinaries() { IValidationSupport validationSupport; validationSupport = mockValidationSupport(); diff --git a/pom.xml b/pom.xml index fdf01913533..88338c65c49 100644 --- a/pom.xml +++ b/pom.xml @@ -944,6 +944,10 @@ plchldr Jonas Beyer + + Boereck + Max Bureck +