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

feat: reduce read syscalls to improve performance #4485

Merged
merged 6 commits into from
Apr 5, 2024

Conversation

lrstewart
Copy link
Contributor

@lrstewart lrstewart commented Apr 2, 2024

Description of changes:

If we want to read one TLS record at a time, we need two reads: one to read the fixed-size header, which includes the total record size, and one to use that record size to read the rest of the record.

However, if we're willing to potentially read more than one record at a time, we can just perform a single optimistic read that attempts to read as much data as we can buffer. This reduces the cost of reads, since reading a record now takes <= 1 read syscall rather than 2 read syscalls. The optimization isn't for free though-- applications need to ensure that they can handle the behavior change.

Testing:

New unit tests.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added the s2n-core team label Apr 2, 2024
@lrstewart lrstewart requested review from camshaft and goatgoose April 3, 2024 01:04
@lrstewart lrstewart marked this pull request as ready for review April 3, 2024 01:04
tests/unit/s2n_recv_greedy_test.c Outdated Show resolved Hide resolved
api/s2n.h Show resolved Hide resolved
tls/s2n_recv.c Outdated Show resolved Hide resolved
@lrstewart lrstewart requested review from camshaft and goatgoose April 4, 2024 05:26
tls/s2n_connection.h Outdated Show resolved Hide resolved
@lrstewart lrstewart enabled auto-merge (squash) April 5, 2024 06:12
@lrstewart lrstewart merged commit b169d76 into aws:main Apr 5, 2024
32 checks passed
@lrstewart lrstewart deleted the recv_buffering branch April 5, 2024 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants