Skip to content

Speed up Deserialization #5237

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Speed up Deserialization #5237

wants to merge 5 commits into from

Conversation

@antonydellavecchia
Copy link
Collaborator

@antonydellavecchia antonydellavecchia commented Aug 22, 2025

Makes progress towards resolving #4558 by using serialization of UUIDs.

Will need an upgrade script so I've disabled upgrade tests for now

@antonydellavecchia antonydellavecchia added serialization optimization Simpler/more performant code or more/better tests release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes labels Aug 22, 2025
@lgoettgens
Copy link
Member

lgoettgens commented Aug 28, 2025

Will need an upgrade script so I've disabled upgrade tests for now

From looking at the changeset, many of the changes don't need an upgrade script. Could you split the changes up into the ones using UUIDs (and thus needing an upgrade script) and the other ones that just improve type stability? I think that way it would be easier to benchmark the different changes to see how much each of them improves the situation

@antonydellavecchia
Copy link
Collaborator Author

antonydellavecchia commented Aug 28, 2025

Honestly, none of these changes currently make any kind of real improvement, even though it seems like it might. So I was thinking of scrapping it, and waiting to hear what are some of Max's ideas on how to improve type stability.

@fingolfin
Copy link
Member

fingolfin commented Oct 29, 2025

@antonydellavecchia actually I did leave some comments on type stability at #4558 (comment), did you see those?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

optimization Simpler/more performant code or more/better tests release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes serialization

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@antonydellavecchia @lgoettgens @fingolfin