stevedlawrence commented on a change in pull request #419:
URL: https://github.com/apache/incubator-daffodil/pull/419#discussion_r488066492
##########
File path:
daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/ElementUnparser.scala
##########
@@ -477,7 +477,7 @@ sealed trait RegularElementUnparserStartEndStrategy
}
val cur = state.currentInfosetNode
if (cur.isComplex)
- cur.asComplex.setFinal()
+ cur.isFinal = true
Review comment:
I think it's actually both a mixture of removeing the trait, and making
things publicly modifable. For example, with a trait we get something like this:
```
public class WithFinalTrait implements Finalizable {
private boolean Finalizable$$_isFinal;
public void Finalizable$$_isFinal_$eq(boolean);
Code:
0: aload_0
1: iload_1
2: putfield #15 // Field Finalizable$$_isFinal:Z
5: return
public void setFinal();
Code:
0: aload_0
1: invokestatic #27 // Method
Finalizable$class.setFinal:(LFinalizable;)V
4: return
}
public abstract class Finalizable$class {
public static void setFinal(Finalizable);
Code:
0: aload_0
1: iconst_1
2: invokeinterface #13, 2 // InterfaceMethod
Finalizable.Finalizable$$_isFinal_$eq:(Z)V
7: return
}
```
So to set the flag we first call setFinal, which calls the static setFinal
method on Finalizable, which then calls \_isFinal_$eq, which finaly sets the
variable to set the value. So three function calls: setFinal, static setFinal,
and isFinal_$eq.
Without the trait, but with private _isFinal, the bytecode is this:
public class WithOutFinalTrait {
private boolean _isFinal;
private void _isFinal_$eq(boolean);
Code:
0: aload_0
1: iload_1
2: putfield #13 // Field _isFinal:Z
5: return
public void setFinal();
Code:
0: aload_0
1: iconst_1
2: invokespecial #22 // Method _isFinal_$eq:(Z)V
5: return
}
So a call to setFinal directly calls _isFinal_$eq to set the variable. So we
remove the one static setFinal call.
With a public final flag and direct modification, the bytecode looks like
this:
public class PublicIsFinal {
private boolean _isFinal;
public boolean _isFinal();
Code:
0: aload_0
1: getfield #13 // Field _isFinal:Z
4: ireturn
public void _isFinal_$eq(boolean);
Code:
0: aload_0
1: iload_1
2: putfield #13 // Field _isFinal:Z
5: return
}
So _isFinal_$eq is called directly which modifies the field directly. So now
we're down to a single function call.
So we go from three functions calls with a trait, to one function call with
public member. This feels like it shouldn't make that much of a difference, but
it definitely did. It wasn't huge, and I'm not sure I would reccommend we do
this everywhere because it does make readability/reusability more difficult,
but in performance critical situations, avoiding traits and private vars seems
to help a bit.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]