Open
Conversation
added 16 commits
May 31, 2023 08:38
Contributor
|
@sigilante will you comment here with your thoughts please? This PR has been open for a month or two now but seems to be stuck. |
Collaborator
|
The code here is fine as far as it fulfills the bounty requirements. The blocker is the jet mismatch because of incorrect atom parsing behavior in Hoon—these jets are more correct and/or require clarification on what we will permit atom behavior to be. We need a decision from core dev on what to do about that, which is I believe addressed in a different issue or gist. |
Contributor
Author
|
The new jets behavior is matched by urbit/urbit#6628. The merge must be coordinated with the other PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introduction
This PR accomplishes the tune slaw/scot jets grant.
Slaw and scot jets were improved and extended, and their performance
benchmarked.
+scotserializes atoms to strings,+slawparses strings to atoms, returning a unit ofthe parsed atom.
The Hoon code of
+scotand+slawwas scrutinized foreach jetted path. The issues found - bugs in the Hoon parser, as well as a jet mismatch, are addressed in a later section.
The work is based on Joe's PR #291, which provided scot jets for %ud, %ux, %uv and %uw.
Scope of the work
In the scot atom printing jet, %ui, %da and %p were added.
(There are existing %da and %p
+scowjets, these however were disabled in December 2020 due to obscure memory errors.)In the slaw parsing jet, auras %da, %p were reimplemented, and %ui, %uv, %uw, %ux were
added.
Unit tests were implemented for all jetted auras, both in Vere, as well as in Arvo tests in the Urbit repo
The scot, slaw aura occurence tables accross desks available in a
standard installation (base, garden, group, landscape and talk) are as follows.
The jet contributions of this PR are marked with a star.
Memory Checks
Past attempts to implement and improve upon slaw/scot jets ended up
hitting memory problems of unknown source, see urbit/urbit#5161. Here, the implemented code was scrutinized carefully, noting the allocation and freeing of each of the introduced variables. Unit testing in
jets_tests.cwithu3m_grabdid not report any memory leaks.Benchmarks
The slaw, scot benchmarks can be found in a Julia notebook: https://mikolajpp.github.io/aura-benchmarks.html
Issues
Incorrect Hoon parser behaviour
During the examination of the relevant Hoon parsing arms several problems were discovered. These are reported in a PR in the Urbit repo: urbit/urbit#6628.
%p parsing jet mismatch
There is a jet mismatch in the existing slaw %p parser.
Below examples should not parse, but with the
+slawjet enabledthey parse on the current runtime.
This is fixed with a rewrite of the
slaw %p jet.
Acknowledgements
I thank @joemfb for making his branch available as a starting point for
this work. I thank @sigilante for his patient mentoring on this project.