Skip to content

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

Merged
merged 24 commits into from
Jul 16, 2025
Merged

Conversation

runarorama
Copy link
Contributor

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.

@runarorama runarorama marked this pull request as draft July 10, 2025 18:55
@runarorama runarorama marked this pull request as ready for review July 14, 2025 03:03
@runarorama runarorama requested review from dolio and pchiusano July 14, 2025 03:03
@aryairani
Copy link
Contributor

aryairani commented Jul 14, 2025

now that stack.yaml has been reverted, just need to fix the build errors.
./scripts/check.sh is a good resource for preparing a happy commit

don't worry about how you resolve the merge conflicts in all-base-hashes.output.md, that file will be regenerated anyway

Copy link
Contributor

@aryairani aryairani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

Copy link
Contributor

@dolio dolio left a 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.

Copy link
Contributor

@dolio dolio left a 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.

@aryairani aryairani merged commit 87f8c02 into trunk Jul 16, 2025
31 checks passed
@aryairani aryairani deleted the runarorama/avroMagic branch July 16, 2025 04:07
@@ -80,7 +80,7 @@ structural type SomethingUnusuallyLong
= SomethingUnusuallyLong Text Text Text
structural type UUID
= UUID Nat (Nat, Nat)
= UUUID Nat (Nat, Nat)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this intentional?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants