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

Aor jet #690

Merged
merged 14 commits into from
Sep 25, 2024
Merged

Aor jet #690

merged 14 commits into from
Sep 25, 2024

Conversation

Quodss
Copy link
Contributor

@Quodss Quodss commented Jul 31, 2024

++aor jet. 10x speed up with this input:

(aor 'aaaaaaaaaaaaaaaa' 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa')

@Quodss Quodss requested a review from a team as a code owner July 31, 2024 10:59
pkg/noun/jets/c/aor.c Outdated Show resolved Hide resolved
pkg/noun/jets/c/aor.c Outdated Show resolved Hide resolved
@Quodss Quodss requested a review from joemfb July 31, 2024 16:18
@Quodss
Copy link
Contributor Author

Quodss commented Jul 31, 2024

Please don't merge it just yet: I want to try adding little-endian optimization

joemfb
joemfb previously approved these changes Jul 31, 2024
Copy link
Member

@joemfb joemfb left a comment

Choose a reason for hiding this comment

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

this will do

@Quodss
Copy link
Contributor Author

Quodss commented Jul 31, 2024

I included portable.h in the jet with an endianness check and an optimized version of the jet. It squizzes out ~17% of performance gains in the case of some ridiculous inputs ([a=(crip (reap 1.000.000 'a')) b=(crip ['a' (reap 1.000.000 'a')])])

Looks like the speed up comes from removing shl and and instructions, which I expected to be optimized away in the first version of the jet.

@Quodss Quodss requested a review from joemfb July 31, 2024 18:39
Copy link
Member

@joemfb joemfb left a comment

Choose a reason for hiding this comment

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

Those #define's are vestigial, and this codebase is little-endian only. So you can just use your little-endian version and drop the other one.

pkg/noun/jets/c/aor.c Outdated Show resolved Hide resolved
@Quodss Quodss requested a review from joemfb August 5, 2024 17:35
pkg/noun/jets/c/aor.c Outdated Show resolved Hide resolved
Copy link
Member

@joemfb joemfb left a comment

Choose a reason for hiding this comment

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

Your naming conventions don't matching u3's suffix patterns. The name suffix should match the type of the variable, regardless of whether it's a pointer: c3_y -> foo_y, c3_w foo_w, &c.

@Quodss Quodss requested a review from joemfb August 5, 2024 17:58
@pkova pkova merged commit ded901f into urbit:develop Sep 25, 2024
@Quodss Quodss deleted the aor-jet branch September 25, 2024 15:19
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.

3 participants