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

Optimize stack usage of Dispatch::poll #1

Merged
merged 1 commit into from
Oct 25, 2023
Merged

Conversation

robin-nitrokey
Copy link
Member

@robin-nitrokey robin-nitrokey commented Oct 6, 2023

Dispatch::poll has to copy the request so that the app can write the response to the interchange. Previously, we called Responder::take_request to obtain the message out of the interchange. This implementation had a stack usage of 15280 bytes while the message size is only 7609 bytes.

With this patch, we only request a reference from the responder and manually copy the message to a buffer. This reduces the stack size to 7664 bytes.

I have no idea why the take_request implementation creates an additional copy of the message. But as this function is the root for all CTAPHID commands, this means we can save around 7 kB stack virtually everywhere. Similar optimizations may be possible in other functions too.

Dispatch::poll has to copy the request so that the app can write the
response to the interchange.  Previously, we called
Responder::take_request to obtain the message out of the interchange.
This implementation had a stack usage of 15280 bytes while the message
size is only 7609 bytes.

With this patch, we only request a reference from the responder and
manually copy the message to a buffer.  This reduces the stack size to
7664 bytes.

I have no idea why the take_request implementation creates an additional
copy of the message.  But as this function is the root for all CTAPHID
commands, this means we can save around 7 kB stack virtually everywhere.
Similar optimizations may be possible in other functions too.
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