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

jets: refactors and optimizes tree-math jets #397

Merged
merged 17 commits into from
Nov 13, 2023
Merged

Conversation

joemfb
Copy link
Member

@joemfb joemfb commented May 17, 2023

Building on urbit/urbit#6592, this PR adds jets for +pin and +hub. Additionally, it fixes some unsafe c3_w -> u3_atom conversions and optimizes +peg and +mas.

This is a draft PR as further testing of these changes is still needed.

/cc @eamsden

pkg/noun/jets/c/hub.c Outdated Show resolved Hide resolved
pkg/noun/jets/c/hub.c Outdated Show resolved Hide resolved
eamsden
eamsden previously approved these changes May 17, 2023
Copy link
Contributor

@eamsden eamsden left a comment

Choose a reason for hiding this comment

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

LGTM now

ashelkovnykov
ashelkovnykov previously approved these changes May 17, 2023
@joemfb
Copy link
Member Author

joemfb commented Oct 7, 2023

If urbit/urbit#6819 is ever restored, 2d696e3 should be reverted.

@joemfb joemfb force-pushed the jb/tree-math-jets branch from c13a351 to de74b25 Compare October 13, 2023 22:57
@joemfb joemfb force-pushed the jb/tree-math-jets branch from 61e7c34 to 562bfac Compare October 14, 2023 01:03
@joemfb joemfb changed the title jets: adds new, optimizes existing tree-math jets jets: refactors and optimizes tree-math jets Oct 14, 2023
@joemfb joemfb marked this pull request as ready for review October 14, 2023 01:04
@joemfb joemfb requested a review from a team as a code owner October 14, 2023 01:04
@joemfb joemfb requested a review from ashelkovnykov October 14, 2023 01:04
@joemfb joemfb requested a review from eamsden November 10, 2023 17:26
u3i_slab sab_u;
u3i_slab_from(&sab_u, a, 0, --b_w);

sab_u.buf_w[(b_w >> 5)] &= ((c3_w)1 << (b_w & 31)) - 1;
Copy link
Member Author

Choose a reason for hiding this comment

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

NB: slab_from rounds up to word boundaries in the slice specified, so we need to mask off any more significant bits than we want.

Comment on lines 36 to 38
sab_u.buf_w[(b_w >> 5)] &= ((c3_w)1 << (b_w & 31)) - 1;
b_w--;
sab_u.buf_w[(b_w >> 5)] |= ((c3_w)1 << (b_w & 31));
Copy link
Contributor

Choose a reason for hiding this comment

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

This mask and store logic is wrong in the boundary condition. @joemfb is going to put masking logic for sub-word blocksizes in u3i_slab_from()

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, this was a great catch

@joemfb
Copy link
Member Author

joemfb commented Nov 11, 2023

I've updated u3i_slab_from() to correctly mask off any unwanted bits from the most significant word when asked to truncate its input atom. All prior call sites used at least the exact word-length of the input atom, so the old behavior was correct in all those cases. But it the API was riskier than needed (and my local work around for it was wrong).

I've corrected the +mas jet, and added some trivial tests to sanity-check the boundary condition.

@pkova pkova merged commit 45d28f9 into develop Nov 13, 2023
5 checks passed
@pkova pkova deleted the jb/tree-math-jets branch November 13, 2023 16:37
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.

4 participants