Skip to content

Conversation

vladem
Copy link
Contributor

@vladem vladem commented Aug 19, 2024

Description of change

To optimize random reads we don't start the second request until half of the first one was read as the second request may not be needed. This results in a significant improvement of read throughput for random reads, which is most noticeable for express:

Relevant issues: N/A

Does this change impact existing behavior?

It doesn't change behavior but impacts performance of random reads.

Does this change need a changelog entry in any of the crates?

No changelog entry required.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I agree to the terms of the Developer Certificate of Origin (DCO).

@vladem vladem had a problem deploying to PR integration tests August 19, 2024 11:48 — with GitHub Actions Failure
@vladem vladem had a problem deploying to PR integration tests August 19, 2024 11:48 — with GitHub Actions Failure
@vladem vladem had a problem deploying to PR integration tests August 19, 2024 11:48 — with GitHub Actions Failure
@vladem vladem had a problem deploying to PR integration tests August 19, 2024 11:48 — with GitHub Actions Failure
@vladem vladem had a problem deploying to PR integration tests August 19, 2024 11:48 — with GitHub Actions Failure
@vladem vladem had a problem deploying to PR integration tests August 19, 2024 11:48 — with GitHub Actions Failure
@vladem vladem had a problem deploying to PR integration tests August 19, 2024 11:48 — with GitHub Actions Failure
@vladem vladem added the performance PRs to run benchmarks on label Aug 19, 2024
@vladem vladem temporarily deployed to PR integration tests August 19, 2024 21:52 — with GitHub Actions Inactive
@vladem vladem temporarily deployed to PR integration tests August 19, 2024 21:52 — with GitHub Actions Inactive
@vladem vladem temporarily deployed to PR integration tests August 19, 2024 21:52 — with GitHub Actions Inactive
@vladem vladem temporarily deployed to PR integration tests August 19, 2024 21:53 — with GitHub Actions Inactive
@vladem vladem temporarily deployed to PR integration tests August 19, 2024 21:53 — with GitHub Actions Inactive
@vladem vladem temporarily deployed to PR integration tests August 19, 2024 21:53 — with GitHub Actions Inactive
@vladem vladem temporarily deployed to PR integration tests August 19, 2024 21:53 — with GitHub Actions Inactive
@vladem vladem changed the title Trigger benchmarks wait_for_read_window_increment after last byte has arrived Aug 21, 2024
Signed-off-by: Vlad Volodkin <vlaad@amazon.com>
@vladem vladem temporarily deployed to PR integration tests August 21, 2024 16:23 — with GitHub Actions Inactive
@vladem vladem temporarily deployed to PR integration tests August 21, 2024 16:23 — with GitHub Actions Inactive
@vladem vladem temporarily deployed to PR integration tests August 21, 2024 16:23 — with GitHub Actions Inactive
@vladem vladem temporarily deployed to PR integration tests August 21, 2024 16:23 — with GitHub Actions Inactive
@vladem vladem temporarily deployed to PR integration tests August 21, 2024 16:23 — with GitHub Actions Inactive
@vladem vladem temporarily deployed to PR integration tests August 21, 2024 16:23 — with GitHub Actions Inactive
@vladem vladem changed the title wait_for_read_window_increment after last byte has arrived Start second S3 request only if required Aug 21, 2024
@vladem vladem changed the title Start second S3 request only if required Start the second S3 request only if required Aug 21, 2024
@vladem vladem marked this pull request as ready for review August 21, 2024 19:58
Copy link
Contributor

@passaro passaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Nice explanation and diagram!

@vladem vladem added this pull request to the merge queue Aug 22, 2024
Merged via the queue into awslabs:main with commit 2cb9c72 Aug 22, 2024
@vladem vladem deleted the bench branch August 22, 2024 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance PRs to run benchmarks on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants