awslabs / aws-c-s3

C99 library implementation for communicating with the S3 service, designed for maximizing throughput on high bandwidth EC2 instances.
Apache License 2.0
93 stars 37 forks source link

Buffer-pool allows "forced" buffers, which don't count against memory limit #429

Closed graebm closed 3 months ago

graebm commented 3 months ago

Issue: aws_s3_meta_request_write() must write to a buffer immediately if the data is less than part-size. Currently, it uses a buffer hooked up the to default allocator (code here). We'd like to get these buffers from the buffer-pool, to reduce memory fragmentation and resident set size (RSS).

The problem is: the buffer-pool maintains strict memory limits, and won't allow further allocations when that limit is hit. But aws_s3_meta_request_write() MUST get a buffer immediately, or else the system can deadlock (see description in PR https://github.com/awslabs/aws-c-s3/pull/418)

Description of changes: Add aws_s3_buffer_pool_acquire_forced_buffer(). These buffers can be created even if they exceed the memory limit.

Future work: Modify aws_s3_meta_request_write() to use this new function

Additional thoughts:

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

codecov-commenter commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 89.54%. Comparing base (cc06c41) to head (f1390ae).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/awslabs/aws-c-s3/pull/429/graphs/tree.svg?width=650&height=150&src=pr&token=J4KP54FVLF&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=awslabs)](https://app.codecov.io/gh/awslabs/aws-c-s3/pull/429?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=awslabs) ```diff @@ Coverage Diff @@ ## main #429 +/- ## ========================================== + Coverage 89.50% 89.54% +0.03% ========================================== Files 20 20 Lines 6015 6036 +21 ========================================== + Hits 5384 5405 +21 Misses 631 631 ``` | [Files](https://app.codecov.io/gh/awslabs/aws-c-s3/pull/429?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=awslabs) | Coverage Δ | | |---|---|---| | [source/s3\_buffer\_pool.c](https://app.codecov.io/gh/awslabs/aws-c-s3/pull/429?src=pr&el=tree&filepath=source%2Fs3_buffer_pool.c&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=awslabs#diff-c291cmNlL3MzX2J1ZmZlcl9wb29sLmM=) | `97.17% <100.00%> (+0.38%)` | :arrow_up: |
DmitriyMusatkin commented 3 months ago

also can you add a section on forced buffers to https://github.com/awslabs/aws-c-s3/blob/main/docs/memory_aware_request_execution.md. we probably want to keep it up to date