diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/core/compiler/Compiler.scala b/daffodil-core/src/main/scala/org/apache/daffodil/core/compiler/Compiler.scala index 1984c8968d..78db91234d 100644 --- a/daffodil-core/src/main/scala/org/apache/daffodil/core/compiler/Compiler.scala +++ b/daffodil-core/src/main/scala/org/apache/daffodil/core/compiler/Compiler.scala @@ -144,7 +144,7 @@ final class ProcessorFactory private ( codeGenerator } - override def isError: Boolean = sset.isError + override lazy val isError: Boolean = sset.isError def withDistinguishedRootNode(name: String, namespace: String): ProcessorFactory = { Assert.usage(name ne null) @@ -378,8 +378,13 @@ class Compiler private ( tunables ) } - pf.isError // call here to drive compilation, so that compilation at least at the level of the PF - // is over when we return the pf. + // It is tempting to call pf.isError here to drive compilation to completion before we + // return the pf to the caller. + // However, this slows down TDML-based testing in Daffodil substantially by moving + // the entire isError checking pass inside the synchronized block of the Daffodil + // schema compiler. This results in reduced concurrency which substantially slows + // the daffodil test suite. + // pf.isError // don't call this here. Call it outside the synchronized block. pf } @@ -410,9 +415,16 @@ object Compiler { optRootName: Option[String], optRootNamespace: Option[String] ): ProcessorFactory = { - synchronized { + val pf = synchronized { c.compileSourceInternal(schemaSource, optRootName, optRootNamespace) } + // Force all compilation to complete. Called here outside of synchronized block on purpose + // to avoid over-serializing things (which would slow down large test suites like Daffodil's test suite.) + // Note that this requires that the shared data structures which require Daffodil schema compilation to + // be serialized do *not* include the data structures being modified during isError processing (which is + // lots of OOLAG evaluations). + pf.isError + pf } } diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SchemaSet.scala b/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SchemaSet.scala index 92d7670f6d..30a06ea47c 100644 --- a/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SchemaSet.scala +++ b/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SchemaSet.scala @@ -683,7 +683,7 @@ final class SchemaSet private ( * and finally the AST objects are checked for errors, which recursively * demands that all other aspects of compilation occur. */ - override def isError: Boolean = { + override lazy val isError: Boolean = { if (!isValid) true else if ( // use keepGoing so we can capture errors and @@ -694,6 +694,15 @@ final class SchemaSet private ( } ) true else { + // we must check for errors here via the super method, as that iterates over + // all the objects evaluating them for any errors in their required evaluations. + // This has to be after everything else that could report an error here (on some + // other object) has been done. + // + // That is to say, if we called super.isError at the top of this method that would + // be incorrect since isValid and areComponentsConstructed above might cause errors + // to be recorded or objects created that this call to super.isError would then not + // take into account. val hasErrors = super.isError if (!hasErrors) { // must be called after compilation is done diff --git a/daffodil-lib/src/main/scala/org/apache/daffodil/lib/oolag/OOLAG.scala b/daffodil-lib/src/main/scala/org/apache/daffodil/lib/oolag/OOLAG.scala index 9bf64780f5..31856bff71 100644 --- a/daffodil-lib/src/main/scala/org/apache/daffodil/lib/oolag/OOLAG.scala +++ b/daffodil-lib/src/main/scala/org/apache/daffodil/lib/oolag/OOLAG.scala @@ -478,16 +478,13 @@ object OOLAG { } /** - * Currently we depend on being able to evaluate these - * repeatedly, and get different answers. - * - * because it forces evaluation of all the requiredEvaluationsAlways(...) + * Forces evaluation of all the requiredEvaluationsAlways(...) * or requiredEvaluationsIfActivated(...) * on all objects first, but that is only for the objects * that have been created and activated at the time this is called. */ - - def isError: Boolean = { + def isError: Boolean = isErrorOnce + private lazy val isErrorOnce: Boolean = { oolagRoot.checkErrors val errorCount = oolagRoot.errors.size errorCount > 0