stevedlawrence commented on code in PR #1097:
URL: https://github.com/apache/daffodil/pull/1097#discussion_r1367294476
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/VariableMap1.scala:
##########
@@ -311,31 +306,33 @@ class VariableMap private (vTable: Map[GlobalQName,
ArrayBuffer[VariableInstance
* expression
*/
def setFirstInstanceInitialValues(): Unit = {
- vTable.foreach {
- case (_, variableInstances) => {
- variableInstances(0).firstInstanceInitialValue =
variableInstances(0).value
- }
+ vTable.foreach { variableInstances =>
+ variableInstances.head.firstInstanceInitialValue =
variableInstances.head.value
}
}
+ /**
+ * Performance of this is linear in number of variables, this should not be
used in
+ * performance critical sections.
+ */
def find(qName: GlobalQName): Option[VariableInstance] = {
- val optBuf = vTable.get(qName)
- val variab = {
- if (optBuf.isDefined)
- Some(optBuf.get.last)
- else
- None
- }
- variab
+ getVariableRuntimeData(qName).map { vrd => vTable(vrd.vmapIndex).head }
Review Comment:
Yeah, I considered doing that, but the only places we do a qname lookup are
in layer compilation (if they use variables), CLI debugging, or setting
external variables. Only that last one *really* matters for performance, but 1)
setting external variables is fairly rare and 2) setExternalVariables has more
compliated logic than a hash lookup--it considers local name, detects
ambiguities if there's no namspace, etc. As you point out linear search is
probably fine for these cases with the usual few number of variables.
In reality, these functions probably aren't that slow, but I added the
comment because I don't want us to accidentally use them in actual
parsing/unparsing logic or something where speed really is critical.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
