Skip to content

Conversation

@mikolajpp
Copy link
Contributor

Introduction

This PR accomplishes the tune slaw/scot jets grant.
Slaw and scot jets were improved and extended, and their performance
benchmarked.

+scot serializes atoms to strings, +slaw parses strings to atoms, returning a unit of
the parsed atom.

> (scot %p 123)
~.rux

> (slaw %ux '0xcafe')
[~ 51.966]

> (slaw %ux '0xcaf.e')
~

The Hoon code of +scot and +slaw was scrutinized for
each 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 +scow jets, 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.

aura scot jet
⭐ %p 444
⭐ %da 281
%ud 144
%uv 123
%ux 56
%uw 31
⭐ %ui 30
%tas 7
%q 5
%t 3
%if 3
%dr 3
%x 1
%rs 1
aura slaw jet
⭐ %p 53
⭐%da 22
%ud 16
⭐%ux 15
⭐%uw 7
⭐%uv 6
%t 4
⭐%ui 3
%tas 2

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.c with u3m_grab did 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 +slaw jet enabled
they parse on the current runtime.

> (slaw %p '~dozzod')
[~ 0]
> (slaw %p '~doznec')
[~ 1]
> (slaw %p '~doznec-litsem')
[~ 1.627.693.323]
> (slaw %p '~dozzod-doznec-litsem')
[~ 1.627.693.323]

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.

@mikolajpp mikolajpp requested a review from a team as a code owner June 1, 2023 02:16
@mikolajpp mikolajpp changed the title Tune slaw, scot jets jets: tune slaw, scot jets Jun 8, 2023
@jalehman jalehman requested review from barter-simsum and joemfb June 26, 2023 15:53
@matthew-levan
Copy link
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.

@sigilante
Copy link
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.

@mikolajpp
Copy link
Contributor Author

The new jets behavior is matched by urbit/urbit#6628. The merge must be coordinated with the other PR.

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

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

3 participants