-
Notifications
You must be signed in to change notification settings - Fork 278
Add Avro-decoder builtin and replacement function #5799
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
Conversation
…rorama/avroMagic
now that don't worry about how you resolve the merge conflicts in |
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.
Nice
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.
Largely looks good to me. Just saw some cleanup stuff.
I can't really verify the Avro details of course. But from conversations we had, it sounds like you have tests that were catching problems with the implementation.
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.
Looks good now, I think.
@@ -80,7 +80,7 @@ structural type SomethingUnusuallyLong | |||
= SomethingUnusuallyLong Text Text Text | |||
structural type UUID | |||
= UUID Nat (Nat, Nat) | |||
= UUUID Nat (Nat, Nat) |
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.
Was this intentional?
Overview
This adds a replacement function for decoding Avro binaries, speeding up Avro decoding by 50x or more.
Implementation notes
Essentially this works like the Json encoding/decoding replacement functions, just with more code.
Interesting/controversial decisions
Everything is just in a section of the relevant files, much like the Json replacement functions. Pulling these out into their own modules turned out to be nontrivial without introducing circular dependencies and the like.
Test coverage
Tests are separately implemented in Unison.