stevedlawrence commented on code in PR #1097:
URL: https://github.com/apache/daffodil/pull/1097#discussion_r1367284096
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/VariableMap1.scala:
##########
@@ -257,26 +269,18 @@ class VariableMap private (vTable: Map[GlobalQName,
ArrayBuffer[VariableInstance
* VariableInstances are mutable and cannot safely be shared across threads
*/
def copy(): VariableMap = {
- val table = vTable.map {
- case (k: GlobalQName, variableInstances: ArrayBuffer[VariableInstance])
=> {
- val newBuf = variableInstances.map { _.copy() }
- (k, newBuf)
- }
+ val table = vTable.map { variableInstances =>
+ val newBuf = variableInstances.map { _.copy() }
+ newBuf
}
- new VariableMap(table)
+ new VariableMap(vrds, table)
}
- def topLevelInstances(): VariableMap = {
- val table = vTable.map {
- case (k: GlobalQName, variableInstances: ArrayBuffer[VariableInstance])
=> {
- val newBuf = new ArrayBuffer[VariableInstance]()
- newBuf.append(variableInstances.last)
- (k, newBuf)
- }
- }
-
- new VariableMap(table)
+ def cloneForSuspension(): VariableMap = {
+ val newTable = new Array[Seq[VariableInstance]](vTable.size)
+ Array.copy(vTable, 0, newTable, 0, vTable.size)
+ new VariableMap(vrds, newTable)
Review Comment:
Yeah, I imagine there's room for improvement. The key issue is that
suspensions currently need a pointer to the current instance of each variable,
so that if later newVariableInstances happen, then those suspensions don't see
that. And we need it for all variables since we don't know which variables a
suspension will need.
We technically don't need the entire stack like this does, we really just
need the top of the stack. So we could have done something like this:
```scala
val newTable = vTable.map { instances => Seq(instances.head) }
new VariableMap(vrds, newTable)
```
But that allocates another Seq, and avoiding allocations was the main
purpose of this. And as long as suspensions aren't pushing/popping variable
instances (which I don't think they should be able to do), this should be fine.
--
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]
