You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by "mbeckerle (via GitHub)" <gi...@apache.org> on 2024/12/20 16:57:20 UTC
[PR] Improve schema compiler performance [daffodil]
mbeckerle opened a new pull request, #1396:
URL: https://github.com/apache/daffodil/pull/1396
For now I have left in the TimeTracker.trace calls that measured the performance.
I will take those out later.
With this change, schema compilation for
Link16 drops from 2+ minutes to 18 seconds.
Added instrumentation to compiler which shows
that most time (by far, like 80+ percent) is
spent in Xerces validating each schema
file individually. This is unnecessary. We
only need to validate the top level schema
to detect UPA errors.
Fixed other error causing repeated
evaluation of OOLAG LV values.
DAFFODIL-2965, DAFFODIL-2781
This is the output from the instrumentation before the functional improvements in this change set:
```
[error] [info] validateAsDFDLSchema schemaFactory.newSchema 66.852 59.13% 67187944 995
[error] [info] resolveCommon 9.720 8.60% 17884 543499
[error] [info] loadDocument 6.607 5.84% 6639796 995
[error] [info] validateAsDFDLSchema load 5.858 5.18% 5887291 995
[error] [info] parserFromURI 5.689 5.03% 5717234 995
[error] [info] unparser 3.707 3.28% 1853341986 2
[error] [info] TypeBase 2.866 2.53% 431697 6639
[error] [info] constructingLoader.load 1.705 1.51% 856144 1991
[error] [info] elementRuntimeData 1.651 1.46% 248657 6638
[error] [info] areComponentsConstructed 1.603 1.42% 1602879249 1
[error] [info] compileSourceInternal 1.018 0.90% 1018485705 1
[error] [info] groupMembers 0.628 0.56% 72189 8701
[error] [info] validateXML 0.605 0.54% 608347 995
```
Here is the instrumentation after:
```
[error] [info] unparser 3.199 21.79% 1599749444 2
[error] [info] TypeBase 1.972 13.43% 297091 6639
[error] [info] areComponentsConstructed 1.696 11.55% 1696149699 1
[error] [info] elementRuntimeData 1.389 9.46% 209236 6638
[error] [info] constructingLoader.load 0.984 6.70% 988328 996
[error] [info] groupMembers 0.646 4.40% 74285 8701
[error] [info] parser 0.490 3.34% 244993274 2
[error] [info] expr 0.427 2.91% 568419 751
[error] [info] DFDLNewVariableInstance_requiredEvaluation_1 0.246 1.68% 2196754 112
[error] [info] compileSourceInternal 0.227 1.55% 227499753 1
[error] [info] repTypeMaps 0.217 1.47% 410835 527
[error] [info] iiSchemaFile 0.197 1.34% 198092 996
[error] [info] encoding 0.177 1.20% 13484 13099
[error] [info] shortFormProperties 0.161 1.10% 7972 20201
[error] [info] nonDefaultPropertySources 0.160 1.09% 8622 18537
[error] [info] resolvedSchemaLocation 0.159 1.08% 40757 3907
[error] [info] notSeenThisBefore 0.141 0.96% 36013 3907
[error] [info] SequenceGroupRef_requiredEvaluation_7 0.124 0.85% 29451 4225
[error] [info] enclosingElements 0.109 0.74% 8547 12726
[error] [info] priorAlignmentApprox 0.099 0.67% 8399 11755
[error] [info] combinedJustThisOneOproperties 0.096 0.65% 4748 20201
[error] [info] longFormProperties 0.091 0.62% 4490 20201
[error] [info] LocalElementDecl_requiredEvaluation_24 0.085 0.58% 13942 6105
[error] [info] defaultPropertySources 0.076 0.52% 4112 18537
[error] [info] hiddenGroupRefOption 0.074 0.50% 13526 5467
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve schema compiler performance [daffodil]
Posted by "jadams-tresys (via GitHub)" <gi...@apache.org>.
jadams-tresys commented on code in PR #1396:
URL: https://github.com/apache/daffodil/pull/1396#discussion_r1896167658
##########
daffodil-japi/src/main/scala/org/apache/daffodil/japi/Daffodil.scala:
##########
@@ -298,7 +298,9 @@ class ProcessorFactory private[japi] (private var pf: SProcessorFactory)
*/
def onPath(path: String) = {
val dp = pf.onPath(path).asInstanceOf[SDataProcessor]
- new DataProcessor(dp)
+ val res = new DataProcessor(dp)
+ res.isError // ensure all errors have been detected before we return the DP
Review Comment:
Makes sense.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve schema compiler performance [daffodil]
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on PR #1396:
URL: https://github.com/apache/daffodil/pull/1396#issuecomment-2557689343
All tests pass on my system.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve schema compiler performance [daffodil]
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on PR #1396:
URL: https://github.com/apache/daffodil/pull/1396#issuecomment-2557441323
Note that this change causes numerous test failures that I'm still working on.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve schema compiler performance [daffodil]
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle merged PR #1396:
URL: https://github.com/apache/daffodil/pull/1396
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Improve schema compiler performance [daffodil]
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on PR #1396:
URL: https://github.com/apache/daffodil/pull/1396#issuecomment-2557463113
Many of the test failures are because diagnostic behavior has changed and now some other SDE or invalid schema aspect is being detected first, before the one the test is expecting.
testSchemaValidationSubset
testScalaAPI26
testJavaAPI27
testTDMLWithInvalidDFDLSchemaEmbedded
test_whitespaceSurroundingErrorMessages
test_multiple_diagnostics1
test_multiple_diagnostics2
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve schema compiler performance [daffodil]
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on PR #1396:
URL: https://github.com/apache/daffodil/pull/1396#issuecomment-2557386851
This is going to produce absurd volume of messages when automated tests are run. So I'm cancelling those for now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve schema compiler performance [daffodil]
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #1396:
URL: https://github.com/apache/daffodil/pull/1396#discussion_r1894192199
##########
build.sbt:
##########
@@ -231,7 +231,7 @@ val minSupportedJavaVersion: String =
lazy val commonSettings = Seq(
organization := "org.apache.daffodil",
- version := "4.0.0-SNAPSHOT",
+ version := "3.10.0-SNAPSHOT",
Review Comment:
Separate PR for this change. DAFFODIL-2966
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve schema compiler performance [daffodil]
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #1396:
URL: https://github.com/apache/daffodil/pull/1396#discussion_r1895863485
##########
daffodil-japi/src/main/scala/org/apache/daffodil/japi/Daffodil.scala:
##########
@@ -298,7 +298,9 @@ class ProcessorFactory private[japi] (private var pf: SProcessorFactory)
*/
def onPath(path: String) = {
val dp = pf.onPath(path).asInstanceOf[SDataProcessor]
- new DataProcessor(dp)
+ val res = new DataProcessor(dp)
+ res.isError // ensure all errors have been detected before we return the DP
Review Comment:
Yes, the idea isn't to handle the errors here as the API user may have their own ideas about when/how to do that. Ex: they may just have an `assert(!pf.isError)` and just fail out if there are errors because they are expecting to use well-known-good schemas only.
The purpose of this is just to force all the lazy evaluation to happen before we return so that we can time things meaningfully. There's no advantage to the lazy evaluation continuing past this point, so we force it all.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve schema compiler performance [daffodil]
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #1396:
URL: https://github.com/apache/daffodil/pull/1396#discussion_r1894374259
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/compiler/Compiler.scala:
##########
@@ -409,9 +410,10 @@ object Compiler {
optRootName: Option[String],
optRootNamespace: Option[String]
): ProcessorFactory = {
- synchronized {
+ val res = synchronized {
Review Comment:
Revert
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/runtime1/SchemaSetRuntime1Mixin.scala:
##########
@@ -48,22 +48,25 @@ trait SchemaSetRuntime1Mixin {
}.value
lazy val parser = LV('parser) {
- val par = if (generateParser) root.document.parser else new NotParsableParser(root.erd)
+ val par =
+ if (generateParser) root.document.parser
+ else new NotParsableParser(root.erd)
Processor.initialize(par)
par
}.value
lazy val unparser = LV('unparser) {
val unp =
- if (generateUnparser) root.document.unparser else new NotUnparsableUnparser(root.erd)
+ if (generateUnparser) root.document.unparser
+ else new NotUnparsableUnparser(root.erd)
Processor.initialize(unp)
unp
}.value
private lazy val layerRuntimeCompiler = new LayerRuntimeCompiler
- lazy val allLayers: Seq[LayerRuntimeData] = LV('allLayers) {
- val lrds: Seq[LayerRuntimeData] = self.allSchemaComponents
+ private lazy val allLayers: Seq[LayerRuntimeData] = LV('allLayers) {
+ lazy val lrds: Seq[LayerRuntimeData] = self.allSchemaComponents
Review Comment:
Revert
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve schema compiler performance [daffodil]
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on PR #1396:
URL: https://github.com/apache/daffodil/pull/1396#issuecomment-2559930592
> There seems to be a regression somewhere in this commit that is costing us concurrency in when running the test suite. Nothing is jumping out at me, but comparing this commit to the main branch, running the full test suite takes roughly 3x as long on this branch than on main.
Yeah, I think I see where this happened. isError is not a lazy val. A bunch of work is done each time it is called. Most of that work will just be finding out all the various things have already been evaluated, but there is substantial overhead just traversing all the objects to find that out. I will investigate further.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve schema compiler performance [daffodil]
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on PR #1396:
URL: https://github.com/apache/daffodil/pull/1396#issuecomment-2560053295
@jadams-tresys the most recent commit addresses the slow-down of the daffodil test suite.
Was due to isError being called inside the synchronized block for the schema compiler. This was unnecessary. Calling isError outside the synchronized block restores the parallelism the loss of which was responsible for the slow-down, or at least that's my story and I'm sticking with it :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve schema compiler performance [daffodil]
Posted by "jadams-tresys (via GitHub)" <gi...@apache.org>.
jadams-tresys commented on code in PR #1396:
URL: https://github.com/apache/daffodil/pull/1396#discussion_r1895810727
##########
daffodil-japi/src/main/scala/org/apache/daffodil/japi/Daffodil.scala:
##########
@@ -298,7 +298,9 @@ class ProcessorFactory private[japi] (private var pf: SProcessorFactory)
*/
def onPath(path: String) = {
val dp = pf.onPath(path).asInstanceOf[SDataProcessor]
- new DataProcessor(dp)
+ val res = new DataProcessor(dp)
+ res.isError // ensure all errors have been detected before we return the DP
Review Comment:
Is there a reason we don't need to handle any errors detected here? Do we assume whoever calls onPath handles the error?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve schema compiler performance [daffodil]
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #1396:
URL: https://github.com/apache/daffodil/pull/1396#discussion_r1894178523
##########
daffodil-lib/src/main/scala/org/apache/daffodil/lib/oolag/OOLAG.scala:
##########
@@ -687,22 +697,26 @@ object OOLAG {
final lazy val valueAsAny: Any = {
if (hasValue) value_.get
- val res =
- try {
- oolagBefore
- val v = body // good place for a breakpoint
- oolagAfterValue(v.asInstanceOf[AnyRef])
- v
- } catch {
- case npe: NullPointerException => throw npe
- case s: scala.util.control.ControlThrowable => throw s
- case u: UnsuppressableException => throw u
- case e: Error => throw e
- case th: Throwable => oolagCatch(th)
- } finally {
- oolagFinalize
- }
- res
+ else {
Review Comment:
This is the other significant functional improvement.
The improvement here was minor at best. Not measurable really. Still it is more correct now.
##########
daffodil-lib/src/main/scala/org/apache/daffodil/lib/util/Timer.scala:
##########
@@ -30,6 +30,7 @@ object Timer {
case "ms" => nanos / 1000000
case "ns" => nanos
}
+ System.err.println(s"[info] Time${msg}: ${time}${units}")
Review Comment:
Revert changes in this file.
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/DFDLSchemaFile.scala:
##########
@@ -206,23 +207,9 @@ final class DFDLSchemaFile(
private lazy val loader = new DaffodilXMLLoader(errHandler)
lazy val iiXMLSchemaDocument = LV('iiXMLSchemaDocument) {
- val res = makeXMLSchemaDocument(seenBefore, Some(this))
- if (res.isDFDLSchema && sset.shouldValidateDFDLSchemas) {
- //
- // We validate DFDL schemas, only if validation is requested.
- // Some things, tests generally, want to turn this validation off.
- //
- try {
- loader.validateAsDFDLSchema(schemaSource)
Review Comment:
This is the big improvement. There is no need to validate each schema file as a DFDL schema, only the root file. This was being called 995 times for Link16.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org