-
Notifications
You must be signed in to change notification settings - Fork 39
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
u3: optimizes u3i_edit (nock opcode 10) #362
Conversation
This PR came up today during a sync between @joemfb and I. I skimmed it to make sure I understood the motivation and how it spiritually aligns w.r.t. similar noun walks in Ares, but I didn't verify the logic carefully enough to really "review" it. |
This reverts commit a6debdb.
bec84ac
to
e182de8
Compare
I've written a benchmark for this code, and run the various versions of this operation through it. The benchmark constructs a list of
Notably, the mutation optimization (we can edit a cell in place if it is on our road and has a refcount of 1) is only a ~30% improvement with the old implementation, but is a 5-6x improvement in the new. I suspect that the branch reduction being slower is an artifact of this benchmark -- we're almost always editing the tail of a cell (lists associate to the right), making branch prediction very effective. So I've left it in for now, but I'm open to being talked out of that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-reviewed later, and then just now w/ Joe
This PR ports urbit/urbit#6004. It is not urgent.
Benchmarking this against the old implementation would be worthwhile. It might also be worth inlining/decomposing
u3a_is_mutable()
; this is (currently) its only call-site, and the road pointer and seniority predicate cannot change throughout this loop.