-
Notifications
You must be signed in to change notification settings - Fork 8
spike: Test using SIMD to parse coordinates #10
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
base: main
Are you sure you want to change the base?
Conversation
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.
I'm feeling your pain, it takes 54ms on my machine, compared to 20ms for my solution, or 18.5ms for the existing solution
let chars = u8x32::from_array([ | ||
value[0], value[1], value[2], value[3], value[4], value[5], value[6], value[7], value[8], | ||
value[9], 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, | ||
]); |
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.
from my testing, it is faster to cast the values into the final type at this point.
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.
The thing is the idea with shuffling all the needed stuff (x, y and (later on) rgb) into a well-defined position only works with u8 as far as I could find :(
let spaces_bitmask = chars.simd_eq(SIMD_SPACE_CHAR).to_bitmask(); | ||
let newline_bitmask = chars.simd_eq(SIMD_NEWLINE_CHAR).to_bitmask(); | ||
let spaces_bitmask = (spaces_bitmask | newline_bitmask) & SPACES_BITMASK_MASK; |
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.
I'm just doing a digits>=10
on digits, as everything below b'0'
will wrap around, this catches all characters other than digits.
Also a note on types: I tried to change my solution to |
If I follow the docs, and use |
Ahh you're right! Have read that and had it back of my mind, but totally missed that. Thx! |
That kind of motivates me to continue on this and add the rgb parsing as well ^^ |
the following test fails: |
This is the reason why I use |
Yep, I have noticed the failing tests as well. I already have an idea on how to solve this (with no performance penalty), but I did not continue any further until we solved the performance problem. The |
Can be extended to parse rgba when we already have it in our vector. I think the swizzle (shuffle) part is costing most of the performance