New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
adding missing registrations in kryo #4451
Conversation
@@ -43,6 +43,13 @@ public void registerClasses(Kryo kryo) { | |||
kryo.register(SAMRecordToGATKReadAdapter.class, new SAMRecordToGATKReadAdapterSerializer()); | |||
|
|||
kryo.register(SAMRecord.class, new SAMRecordSerializer()); | |||
kryo.register(BAMRecord.class, new SAMRecordSerializer()); | |||
|
|||
kryo.register(SAMFileHeader.class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This collides (admittedly harmlessly, in this case) with a registration for the same class in the ADAM serializer. I think we have to deal with the issue of registration precedence in this branch -- can you find a way to ensure that the GATK registrations always take precedence over any registrations for the same classes in our dependencies?
some classes were missing registration in kryo which causes less efficient serialization adding registrations for a number of classes that MarkDuplicatesSpark needs that weren't registered yet notably, BAMRecord wasn't registered to use the correct serializer which could cause major inefficiencies it's not clear what circumstances we're serializing BAMRecord instead of SAMRecordToGATKReadAdapter so how much this will help is not obvious
… way with ADAM's registration
d0c0d50
to
93d322e
Compare
Codecov Report
@@ Coverage Diff @@
## master #4451 +/- ##
===============================================
+ Coverage 79.116% 79.119% +0.003%
- Complexity 16472 16473 +1
===============================================
Files 1047 1047
Lines 59199 59207 +8
Branches 9676 9676
===============================================
+ Hits 46836 46844 +8
+ Misses 8600 8599 -1
- Partials 3763 3764 +1
|
some classes were missing registration in kryo which causes less efficient serialization
adding registrations for a number of classes that MarkDuplicatesSpark needs that weren't registered yet
notably, BAMRecord wasn't registered to use the correct serializer which could cause major inefficiencies
it's not clear what circumstances we're serializing BAMRecord instead of SAMRecordToGATKReadAdapter so how much this will help is not obvious