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

perf: Change underlying Elems to *const u8 #29089

Merged
merged 2 commits into from
Dec 6, 2022

Conversation

joncinque
Copy link
Contributor

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 just Packet.

Summary of Changes

The underlying perf libs take uint8_t*, so make Elems consistent with that, which will also make it work for another Packet type.

Fixes #

@joncinque joncinque merged commit 504091f into solana-labs:master Dec 6, 2022
@joncinque joncinque deleted the perf-u8 branch December 6, 2022 17:06
gnapoli23 pushed a commit to gnapoli23/solana that referenced this pull request Dec 16, 2022
* perf: Change underlying Elems to *const u8

* Remove clippy allows
Comment on lines +18 to 19
pub elems: *const u8,
pub num: u32,
Copy link
Contributor

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 Packets, 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.

Copy link
Contributor Author

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.

nickfrosty pushed a commit to nickfrosty/solana that referenced this pull request Jan 4, 2023
* perf: Change underlying Elems to *const u8

* Remove clippy allows
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants