Fix performance regression in DSTU3 validation

This commit is contained in:
jamesagnew 2018-04-28 11:42:59 -04:00
parent e299b062a6
commit 83a5eedb29
6 changed files with 164 additions and 66 deletions

View File

@ -1,11 +1,10 @@
package ca.uhn.fhir.jpa.demo;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.jpa.dao.DaoConfig;
import org.apache.commons.cli.ParseException;
import org.apache.commons.lang3.Validate;
import ca.uhn.fhir.jpa.dao.DaoConfig;
import ca.uhn.fhir.context.FhirContext;
public class ContextHolder {
private static boolean ourAllowExternalRefs;
@ -23,47 +22,50 @@ public class ContextHolder {
return ourCtx;
}
public static void setCtx(FhirContext theCtx) throws ParseException {
switch (theCtx.getVersion().getVersion()) {
case DSTU2:
ourPath = "/baseDstu2/";
break;
case DSTU3:
ourPath = "/baseDstu3/";
break;
case R4:
ourPath = "/baseR4/";
break;
default:
throw new ParseException("FHIR version not supported by this command: " + theCtx.getVersion().getVersion());
}
ourCtx = theCtx;
}
public static String getPath() {
Validate.notNull(ourPath, "Context not set");
return ourPath;
}
public static Long getReuseCachedSearchResultsForMillis() {
return ourReuseSearchResultsMillis;
}
public static void setReuseCachedSearchResultsForMillis(Long reuseSearchResultsMillis) {
ourReuseSearchResultsMillis = reuseSearchResultsMillis;
}
public static boolean isAllowExternalRefs() {
return ourAllowExternalRefs;
}
public static boolean isDisableReferentialIntegrity() {
return ourDisableReferentialIntegrity;
}
public static void setAllowExternalRefs(boolean theAllowExternalRefs) {
ourAllowExternalRefs = theAllowExternalRefs;
}
public static void setCtx(FhirContext theCtx) throws ParseException {
switch (theCtx.getVersion().getVersion()) {
case DSTU2:
ourPath = "/baseDstu2/";
break;
case DSTU3:
ourPath = "/baseDstu3/";
break;
default:
throw new ParseException("FHIR version not supported by this command: " + ContextHolder.getCtx().getVersion().getVersion());
}
ourCtx = theCtx;
public static boolean isDisableReferentialIntegrity() {
return ourDisableReferentialIntegrity;
}
public static void setDisableReferentialIntegrity(boolean theDisableReferentialIntegrity) {
ourDisableReferentialIntegrity = theDisableReferentialIntegrity;
}
public static void setReuseCachedSearchResultsForMillis(Long reuseSearchResultsMillis) {
ourReuseSearchResultsMillis = reuseSearchResultsMillis;
}
public static Long getReuseCachedSearchResultsForMillis() {
return ourReuseSearchResultsMillis;
}
}

View File

@ -31,6 +31,11 @@
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>com.github.ben-manes.caffeine</groupId>
<artifactId>caffeine</artifactId>
</dependency>
<!--
Optional dependencies from RI codebase
-->

View File

@ -6,10 +6,15 @@ import ca.uhn.fhir.rest.api.EncodingEnum;
import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
import ca.uhn.fhir.validation.IValidationContext;
import ca.uhn.fhir.validation.IValidatorModule;
import com.github.benmanes.caffeine.cache.CacheLoader;
import com.github.benmanes.caffeine.cache.Caffeine;
import com.github.benmanes.caffeine.cache.LoadingCache;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import com.google.gson.JsonObject;
import org.apache.commons.lang3.Validate;
import org.apache.commons.lang3.builder.EqualsBuilder;
import org.apache.commons.lang3.builder.HashCodeBuilder;
import org.hl7.fhir.convertors.VersionConvertor_30_40;
import org.hl7.fhir.dstu3.hapi.ctx.HapiWorkerContext;
import org.hl7.fhir.dstu3.hapi.ctx.IValidationSupport;
@ -38,6 +43,7 @@ import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import java.io.StringReader;
import java.util.*;
import java.util.concurrent.TimeUnit;
public class FhirInstanceValidator extends BaseValidatorBridge implements IValidatorModule {
@ -49,6 +55,7 @@ public class FhirInstanceValidator extends BaseValidatorBridge implements IValid
private StructureDefinition myStructureDefintion;
private IValidationSupport myValidationSupport;
private boolean noTerminologyChecks = false;
private volatile WorkerContextWrapper myWrappedWorkerContext;
/**
* Constructor
@ -136,6 +143,7 @@ public class FhirInstanceValidator extends BaseValidatorBridge implements IValid
*/
public void setValidationSupport(IValidationSupport theValidationSupport) {
myValidationSupport = theValidationSupport;
myWrappedWorkerContext = null;
}
/**
@ -174,9 +182,16 @@ public class FhirInstanceValidator extends BaseValidatorBridge implements IValid
myStructureDefintion = theStructureDefintion;
}
protected List<ValidationMessage> validate(final FhirContext theCtx, String theInput, EncodingEnum theEncoding) {
HapiWorkerContext workerContext = new HapiWorkerContext(theCtx, myValidationSupport);
WorkerContextWrapper wrappedWorkerContext = new WorkerContextWrapper(workerContext);
WorkerContextWrapper wrappedWorkerContext = myWrappedWorkerContext;
if (wrappedWorkerContext == null) {
HapiWorkerContext workerContext = new HapiWorkerContext(theCtx, myValidationSupport);
wrappedWorkerContext = new WorkerContextWrapper(workerContext);
}
myWrappedWorkerContext = wrappedWorkerContext;
InstanceValidator v;
FHIRPathEngine.IEvaluationContext evaluationCtx = new org.hl7.fhir.r4.hapi.validation.FhirInstanceValidator.NullEvaluationContext();
@ -191,7 +206,7 @@ public class FhirInstanceValidator extends BaseValidatorBridge implements IValid
v.setResourceIdRule(IdStatus.OPTIONAL);
v.setNoTerminologyChecks(isNoTerminologyChecks());
List<ValidationMessage> messages = new ArrayList<ValidationMessage>();
List<ValidationMessage> messages = new ArrayList<>();
if (theEncoding == EncodingEnum.XML) {
Document document;
@ -253,6 +268,42 @@ public class FhirInstanceValidator extends BaseValidatorBridge implements IValid
private final HapiWorkerContext myWrap;
private final VersionConvertor_30_40 myConverter;
private volatile List<org.hl7.fhir.r4.model.StructureDefinition> myAllStructures;
private LoadingCache<ResourceKey, org.hl7.fhir.r4.model.Resource> myFetchResourceCache
= Caffeine.newBuilder()
.expireAfterWrite(10, TimeUnit.SECONDS)
.maximumSize(10000)
.build(new CacheLoader<ResourceKey, org.hl7.fhir.r4.model.Resource>() {
@Override
public org.hl7.fhir.r4.model.Resource load(FhirInstanceValidator.ResourceKey key) throws Exception {
org.hl7.fhir.dstu3.model.Resource fetched;
switch (key.getResourceName()) {
case "StructureDefinition":
fetched = myWrap.fetchResource(StructureDefinition.class, key.getUri());
break;
case "ValueSet":
fetched = myWrap.fetchResource(ValueSet.class, key.getUri());
break;
case "CodeSystem":
fetched = myWrap.fetchResource(CodeSystem.class, key.getUri());
break;
case "Questionnaire":
fetched = myWrap.fetchResource(Questionnaire.class, key.getUri());
break;
default:
throw new UnsupportedOperationException("Don't know how to fetch " + key.getResourceName());
}
if (fetched == null) {
return null;
}
try {
return VersionConvertor_30_40.convertResource(fetched);
} catch (FHIRException e) {
throw new InternalErrorException(e);
}
}
});
public WorkerContextWrapper(HapiWorkerContext theWorkerContext) {
myWrap = theWorkerContext;
@ -372,33 +423,12 @@ public class FhirInstanceValidator extends BaseValidatorBridge implements IValid
@Override
public <T extends org.hl7.fhir.r4.model.Resource> T fetchResource(Class<T> class_, String uri) {
org.hl7.fhir.dstu3.model.Resource fetched;
switch (class_.getSimpleName()) {
case "StructureDefinition":
fetched = myWrap.fetchResource(StructureDefinition.class, uri);
break;
case "ValueSet":
fetched = myWrap.fetchResource(ValueSet.class, uri);
break;
case "CodeSystem":
fetched = myWrap.fetchResource(CodeSystem.class, uri);
break;
case "Questionnaire":
fetched = myWrap.fetchResource(Questionnaire.class, uri);
break;
default:
throw new UnsupportedOperationException("Don't know how to fetch " + class_.getSimpleName());
}
if (fetched == null) {
return null;
}
ResourceKey key = new ResourceKey(class_.getSimpleName(), uri);
@SuppressWarnings("unchecked")
T retVal = (T) myFetchResourceCache.get(key);
try {
return (T) VersionConvertor_30_40.convertResource(fetched);
} catch (FHIRException e) {
throw new InternalErrorException(e);
}
return retVal;
}
@Override
@ -464,6 +494,11 @@ public class FhirInstanceValidator extends BaseValidatorBridge implements IValid
return new HashSet<>(myWrap.getResourceNames());
}
@Override
public org.hl7.fhir.r4.model.StructureMap getTransform(String url) {
throw new UnsupportedOperationException();
}
@Override
public List<String> getTypeNames() {
return myWrap.getTypeNames();
@ -489,6 +524,11 @@ public class FhirInstanceValidator extends BaseValidatorBridge implements IValid
return myWrap.isNoTerminologyServer();
}
@Override
public List<org.hl7.fhir.r4.model.StructureMap> listTransforms() {
throw new UnsupportedOperationException();
}
@Override
public IParser newJsonParser() {
throw new UnsupportedOperationException();
@ -524,16 +564,6 @@ public class FhirInstanceValidator extends BaseValidatorBridge implements IValid
throw new UnsupportedOperationException();
}
@Override
public List<org.hl7.fhir.r4.model.StructureMap> listTransforms() {
throw new UnsupportedOperationException();
}
@Override
public org.hl7.fhir.r4.model.StructureMap getTransform(String url) {
throw new UnsupportedOperationException();
}
@Override
public Set<String> typeTails() {
return myWrap.typeTails();
@ -615,5 +645,52 @@ public class FhirInstanceValidator extends BaseValidatorBridge implements IValid
org.hl7.fhir.dstu3.context.IWorkerContext.ValidationResult result = myWrap.validateCode(system, code, display, conceptSetComponent);
return convertValidationResult(result);
}
}
private static class ResourceKey {
private final int myHashCode;
private String myResourceName;
private String myUri;
private ResourceKey(String theResourceName, String theUri) {
myResourceName = theResourceName;
myUri = theUri;
myHashCode = new HashCodeBuilder(17, 37)
.append(myResourceName)
.append(myUri)
.toHashCode();
}
@Override
public boolean equals(Object theO) {
if (this == theO) {
return true;
}
if (theO == null || getClass() != theO.getClass()) {
return false;
}
ResourceKey that = (ResourceKey) theO;
return new EqualsBuilder()
.append(myResourceName, that.myResourceName)
.append(myUri, that.myUri)
.isEquals();
}
public String getResourceName() {
return myResourceName;
}
public String getUri() {
return myUri;
}
@Override
public int hashCode() {
return myHashCode;
}
}
}

View File

@ -98,6 +98,7 @@
<feature version='${project.version}'>hapi-fhir</feature>
<feature version='${project.version}'>hapi-fhir-utilities</feature>
<feature version='${project.version}'>hapi-fhir-ph-schematron</feature>
<bundle dependency='true'>mvn:com.github.ben-manes.caffeine/caffeine/${caffeine_version}</bundle>
<bundle>mvn:ca.uhn.hapi.fhir/hapi-fhir-converter/${project.version}</bundle>
<bundle>mvn:ca.uhn.hapi.fhir/hapi-fhir-validation/${project.version}</bundle>
</feature>

View File

@ -438,6 +438,7 @@
<!-- Dependency Versions -->
<apache_karaf_version>4.1.4</apache_karaf_version>
<caffeine_version>2.6.2</caffeine_version>
<commons_codec_version>1.10</commons_codec_version>
<commons_io_version>2.5</commons_io_version>
<commons_lang3_version>3.6</commons_lang3_version>
@ -524,6 +525,11 @@
<artifactId>jackson-module-jaxb-annotations</artifactId>
<version>2.9.2</version>
</dependency>
<dependency>
<groupId>com.github.ben-manes.caffeine</groupId>
<artifactId>caffeine</artifactId>
<version>${caffeine_version}</version>
</dependency>
<dependency>
<groupId>com.icegreen</groupId>
<artifactId>greenmail</artifactId>

View File

@ -78,6 +78,13 @@
applies to the operation by name whether it is at the
server, type, or instance level.
</action>
<action type="fix">
Fix a significant performance regression in 3.3.0 when validating DSTU3 content using the
InstanceValidator. From 3.3.0 onward, StructureDefinitions are converted to FHIR R4
content on the fly in order to reduct duplication in the codebase. These conversions
happened upon every validation however, instead of only happening once which adversely
affected performance. A cache has been added.
</action>
</release>
<release version="3.3.0" date="2018-03-29">
<action type="add">