-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
perf: Change underlying Elems to *const u8 #29089
Conversation
* perf: Change underlying Elems to *const u8 * Remove clippy allows
pub elems: *const u8, | ||
pub num: u32, |
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.
@joncinque
Seems like num
field here is the number of Packet
s, not bytes.
So if elems
does not have a known type, num
becomes ambiguous.
e.g. how would the C
code iterate over elems
if their size is unknown.
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.
That's correct that num
refers to the number of elements, and not their size.
If you look at the API that uses Elems
, however, which is ed25519_verify_many
and ed25519_sign_many
, you'll see that we pass message_size
, which defines the size of each packet. In fact, we currently pass in size_of::<Packet>() as u32
for that parameter. So the size is not defined by this type here.
You can even see the underlying function calls in solana-perf-libs
starting at https://github.com/solana-labs/solana-perf-libs/blob/233c4457fd8d425da99cb3db46b2be8eb8b98997/src/cuda-ecc-ed25519/ed25519.h#L35, where they just use a uint_8 *
underneath.
* perf: Change underlying Elems to *const u8 * Remove clippy allows
Problem
As part of the work in #29055 to refactor the
Packet
type, the GPU sigverify path needs to work for two different types, and not justPacket
.Summary of Changes
The underlying perf libs take
uint8_t*
, so makeElems
consistent with that, which will also make it work for anotherPacket
type.Fixes #