Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/13680
I am kind in favor of the single implementation for a couple of reasons:
- Declaring methods `final` is not a magic bullet. If you are
invoking`isNull` or `get*` on the common ancestor it is still a virtual call.
If you are only using two classes the JVM is clever enough to add a branch in
the code and call one of the two methods depending of the type (it is probably
also clever enough to inline the methods). We could get around this if we can
use the concrete type during code generation (which should be doable).
- Nulling out the null region is IMO not really a problem. JVMs nowadays
are very good at optimizing array fill like operations. See this post for some
more information:
http://psy-lob-saw.blogspot.com/2015/04/on-arraysfill-intrinsics-superword-and.html
- On our side code generation should be clever enough not to do null checks
for non-nullable fields (is it?). So we are not incurring useless branch here.
It would help to have a number of performance benchmarks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
