Skip to content
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

Contexts that can be serialized + deserialized while retaining and explicitly representing sharing #2202

Merged
merged 30 commits into from
Nov 22, 2024

Conversation

byorgey
Copy link
Member

@byorgey byorgey commented Nov 5, 2024

Towards #2107. Instead of simply storing a Map, contexts now also store a structured tree representing how it was built, and cache homomorphic hash values so contexts can be compared + stored by hash. This allows us to serialize contexts into a compact form that never duplicates any context, then deserialize them in a way that restores all the sharing.

The big thing that is left is to make FromJSONE instances for Value and Env (and anything which contains them). I was working on these but got bogged down, so decided to not let the perfect be the enemy of the good. You can see some commented-out code towards a FromJSONE Value instance. But we don't use any such instances yet, so I think this can be merged as is, and the rest worked out later.

@byorgey
Copy link
Member Author

byorgey commented Nov 5, 2024

Not quite ready to merge --- still need to update more JSON instances --- but I would be happy to get any comments or questions at this point!

I've tried to put in a bunch of comments but it's quite possible, even likely, that it's still confusing. Please let me know and I will happily add more comments or clarify anything that's confusing.

@byorgey byorgey marked this pull request as ready for review November 19, 2024 21:10
@byorgey byorgey requested a review from xsebek November 19, 2024 21:51
@byorgey byorgey requested a review from kostmo November 19, 2024 21:51
src/swarm-lang/Swarm/Language/Context.hs Outdated Show resolved Hide resolved
src/swarm-lang/Swarm/Language/Context.hs Outdated Show resolved Hide resolved
src/swarm-lang/Swarm/Language/Context.hs Outdated Show resolved Hide resolved
src/swarm-lang/Swarm/Language/Context.hs Show resolved Hide resolved
Copy link
Member

@xsebek xsebek left a comment

Choose a reason for hiding this comment

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

Thanks @byorgey, this enables debugging using the Web API again. 👍

I considered removing the context in JSON output entirely, so having only hashes is better than what I would have aimed for. 😄

@byorgey
Copy link
Member Author

byorgey commented Nov 22, 2024

Thanks @byorgey, this enables debugging using the Web API again. 👍

I considered removing the context in JSON output entirely, so having only hashes is better than what I would have aimed for. 😄

Great! So can we close #2106 ? Edit: just tried it myself, I think the answer is yes.

@byorgey byorgey linked an issue Nov 22, 2024 that may be closed by this pull request
@byorgey byorgey added the merge me Trigger the merge process of the Pull request. label Nov 22, 2024
@mergify mergify bot merged commit 3e05a07 into main Nov 22, 2024
12 checks passed
@mergify mergify bot deleted the ctx-sharing branch November 22, 2024 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Trigger the merge process of the Pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Web API for base robot outputs 800MB per minute
3 participants