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

Prefer cooperative compression over offload to blocking pool. #3153

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

mstyura
Copy link

@mstyura mstyura commented Sep 30, 2023

PR Type

Refactor

PR Checklist

  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • A changelog entry has been made for the appropriate packages.
  • Format code with the latest stable rustfmt.
  • (Team) Label with affected crates and semver status.

Overview

The current implementation of compression middleware offloads compression of chunks larger than 1024 bytes to a blocking pool. Presumably, this was done to avoid preventing other requests from being served, as no other future can make progress due to the CPU-bound operation that compression is.

Threads from the blocking pool do not provide more computational resources than what is already available to runtime polling futures. Therefore, it makes sense to perform compression in chunks by yielding control from the compression future to other futures, allowing everyone to make progress.

I've also observed uncontrollable thread count growth when benchmarking a real-world application with compression of JSON responses enabled:
image
image

@mstyura mstyura marked this pull request as draft October 11, 2023 19:39
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.

None yet

1 participant