Skip to content

Bump Default Memory Limit for Higher Target Throughput #499

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.

Sign up for GitHub

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

Jump to bottom
Merged
waahm7 merged 1 commit into from
Mar 14, 2025
Merged

Bump Default Memory Limit for Higher Target Throughput #499

waahm7 merged 1 commit into from
Mar 14, 2025

Conversation

@waahm7
Copy link
Contributor

@waahm7 waahm7 commented Mar 13, 2025

Description of changes:

  • Bump the memory limit for target_throughput >= 100. While doing testing on trn1.32xlarge with 4 NICs and 400 target_throughput, I noticed that bumping up the memory limit results in a ~11% higher throughput. It also makes 100 Gbps target_throughput more stable. So bump the memory limit while we gather more data on what's going on under the hood.
image Screenshot 2025-03-12 at 11 38 16 AM

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

@codecov-commenter
Copy link

codecov-commenter commented Mar 13, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.64%. Comparing base (aa2d139) to head (5548527).
⚠️ Report is 27 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #499   +/-   ##
=======================================
  Coverage   89.64%   89.64%           
=======================================
  Files          20       20           
  Lines        6344     6346    +2     
=======================================
+ Hits         5687     5689    +2     
  Misses        657      657
Files with missing lines Coverage Δ
source/s3_client.c 91.05% <100.00%> (+0.01%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@waahm7 waahm7 merged commit 169842b into main Mar 14, 2025
35 checks passed
@waahm7 waahm7 deleted the bump-memory-limit branch March 14, 2025 19:02
@passaro passaro mentioned this pull request Apr 1, 2025
Merged
github-merge-queue bot pushed a commit to awslabs/mountpoint-s3 that referenced this pull request Apr 1, 2025
@passaro
Update CRT submodules to latest releases (#1338)
c52ab15 
Update the CRT libraries to the latest releases. Notable changes:
* Update endpoints.
([awslabs/aws-c-s3#502](awslabs/aws-c-s3#502))
* Bump Default Memory Limit for Higher Target Throughput.
([awslabs/aws-c-s3#499](awslabs/aws-c-s3#499))


<details>
  <summary>Full CRT changelog:</summary>
  
```
Submodule mountpoint-s3-crt-sys/crt/aws-c-auth 01dd06ac..cd9d6afc:
  > Update docs for DefaultChain (#266)
  > Async cognito support (#267)
  > only forbid `X-Amz-S3session-Token` when signing with s3 express. (#268)
Submodule mountpoint-s3-crt-sys/crt/aws-c-cal 5d5bc553..4805a96e:
  > Fix FindCrypto behavior on win (#211)
  > Fix module export to respect ed25519 flag (#210)
  > Fix missed define in the code (#209)
Submodule mountpoint-s3-crt-sys/crt/aws-c-common 7fb0071a..8ae8f48e:
  > Simplify how inline math files are included (#1195)
  > Tests require compiler extensions (#1193)
  > CrossProcess lock -- don't unlock, just close fd (#1192)
Submodule mountpoint-s3-crt-sys/crt/aws-c-http 60c43f80..e526ac33:
  > Apple Network Framework Support (#502)
  > h1_decoder error on multiple content-length headers (#509)
  > Fix Error Handling For Connection Manager (#507)
  > HTTP/1: Support streaming requests of unknown length (#506)
Submodule mountpoint-s3-crt-sys/crt/aws-c-io 318f7e57..6c90e491:
  > Remove unused variables in aws_host_resolver (#719)
  > Grand dispatch queue (#661)
  > Fix IP address being labelled "bad" for too long (#718)
  > Add back kqueue support on iOS (#716)
Submodule mountpoint-s3-crt-sys/crt/aws-c-s3 1d0091c7..408e9c90:
  > Update endpoints (#502)
  > Newer URL for aws-lc (#500)
  > Bump Default Memory Limit for Higher Target Throughput (#499)
  > missed one file from test helper README (#498)
Submodule mountpoint-s3-crt-sys/crt/aws-checksums fb8bd0b8..66b447c0:
  > Add missing extern c to new header (#103)
  > Add init functions to support thread safe init of impls (#102)
Submodule mountpoint-s3-crt-sys/crt/aws-lc 7bca7e96..b1420f27:
  > Prepare for v1.49.1 (#2303)
  > Turn on better logging for EC2 test framework (#2298)
  > Add req to OpenSSL CLI tool (#2284)
  > Add more build options to match callback build (#2279)
  > FIPS Integrity Hash Tooling (#2296)
  > Prepare for v1.49.0 (#2297)
  > Cherrypick hardening DSA param checks from BoringSSL  (#2293)
  > Bump mysql CI to 9.2.0 (#2161)
  > AES: Add function pointer trampoline to avoid delocator issue (#2294)
  > Adding detection of out-of-bound pre-bound memory read to AES-XTS tests. (#2286)
  > Wire-up rust-openssl into GitHub CI (for the time being) (#2291)
  > Add support for more SSL BIO functions (#2273)
  > Add support for verifying PKCS7 signed attributes (#2264)
  > Reject DSA trailing garbage in EVP layer, add test cases (#2289)
  > Update patches in Ruby CI (#2233)
  > Documentation on service indicator (#2281)
  > Add the rehash utility to the openssl CLI tool (#2258)
  > Revert "Allow constructed strings in BER parsing (#2015)" (#2278)
  > Prepare AWS-LC v1.48.5 (#2274)
  > s2n-bignum build should use boringssl_prefix_symbols_asm.h (#2265)
  > ci: Nix flake and devShell (#2189)
  > GitHub CI job to verify tags are on expected branches (#2170)
  > Prepare for release v.1.48.4 (#2271)
  > Make AWS_LC_fips_failure_callback optional in builds with AWSLC_FIPS_FAILURE_CALLBACK (#2266)
  > Prepare v1.48.3 (#2269)
  > Update shard_gtest to unset environment variables once all the tests are done (#2270)
  > Minor build fixes for CMake and libssl on x86 (#2267)
  > Fix aws-lc-rs CI test (again) (#2268)
  > Add x4 batched SHAKE128 and SHAKE256 APIs (#2247)
  > Fix aws-lc-rs CI test when symbols removed (#2262)
  > Remove no-op register move from ChaCha20_ctr32_ssse3_4x (#2234)
  > Revert removal of "PEM_X509_INFO_write_bio" (#2226)
  > Use unsigned return type for BN_get_minimal_width and word size tests (#2260)
```
</details>

### Does this change impact existing behavior?

No.

### Does this change need a changelog entry? Does it require a version
change?

Yes. Updated as 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)](https://developercertificate.org/).

---------

Signed-off-by: Alessandro Passaro <[email protected]>
mansi153 pushed a commit to mansi153/mountpoint-s3 that referenced this pull request Jul 24, 2025
@passaro @mansi153
Update CRT submodules to latest releases (awslabs#1338)
ae06df9 
Update the CRT libraries to the latest releases. Notable changes:
* Update endpoints.
([awslabs/aws-c-s3#502](awslabs/aws-c-s3#502))
* Bump Default Memory Limit for Higher Target Throughput.
([awslabs/aws-c-s3#499](awslabs/aws-c-s3#499))


<details>
  <summary>Full CRT changelog:</summary>
  
```
Submodule mountpoint-s3-crt-sys/crt/aws-c-auth 01dd06ac..cd9d6afc:
  > Update docs for DefaultChain (awslabs#266)
  > Async cognito support (awslabs#267)
  > only forbid `X-Amz-S3session-Token` when signing with s3 express. (awslabs#268)
Submodule mountpoint-s3-crt-sys/crt/aws-c-cal 5d5bc553..4805a96e:
  > Fix FindCrypto behavior on win (awslabs#211)
  > Fix module export to respect ed25519 flag (awslabs#210)
  > Fix missed define in the code (awslabs#209)
Submodule mountpoint-s3-crt-sys/crt/aws-c-common 7fb0071a..8ae8f48e:
  > Simplify how inline math files are included (awslabs#1195)
  > Tests require compiler extensions (awslabs#1193)
  > CrossProcess lock -- don't unlock, just close fd (awslabs#1192)
Submodule mountpoint-s3-crt-sys/crt/aws-c-http 60c43f80..e526ac33:
  > Apple Network Framework Support (awslabs#502)
  > h1_decoder error on multiple content-length headers (awslabs#509)
  > Fix Error Handling For Connection Manager (awslabs#507)
  > HTTP/1: Support streaming requests of unknown length (awslabs#506)
Submodule mountpoint-s3-crt-sys/crt/aws-c-io 318f7e57..6c90e491:
  > Remove unused variables in aws_host_resolver (awslabs#719)
  > Grand dispatch queue (awslabs#661)
  > Fix IP address being labelled "bad" for too long (awslabs#718)
  > Add back kqueue support on iOS (awslabs#716)
Submodule mountpoint-s3-crt-sys/crt/aws-c-s3 1d0091c7..408e9c90:
  > Update endpoints (awslabs#502)
  > Newer URL for aws-lc (awslabs#500)
  > Bump Default Memory Limit for Higher Target Throughput (awslabs#499)
  > missed one file from test helper README (awslabs#498)
Submodule mountpoint-s3-crt-sys/crt/aws-checksums fb8bd0b8..66b447c0:
  > Add missing extern c to new header (awslabs#103)
  > Add init functions to support thread safe init of impls (awslabs#102)
Submodule mountpoint-s3-crt-sys/crt/aws-lc 7bca7e96..b1420f27:
  > Prepare for v1.49.1 (#2303)
  > Turn on better logging for EC2 test framework (#2298)
  > Add req to OpenSSL CLI tool (#2284)
  > Add more build options to match callback build (#2279)
  > FIPS Integrity Hash Tooling (#2296)
  > Prepare for v1.49.0 (#2297)
  > Cherrypick hardening DSA param checks from BoringSSL  (#2293)
  > Bump mysql CI to 9.2.0 (#2161)
  > AES: Add function pointer trampoline to avoid delocator issue (#2294)
  > Adding detection of out-of-bound pre-bound memory read to AES-XTS tests. (#2286)
  > Wire-up rust-openssl into GitHub CI (for the time being) (#2291)
  > Add support for more SSL BIO functions (#2273)
  > Add support for verifying PKCS7 signed attributes (#2264)
  > Reject DSA trailing garbage in EVP layer, add test cases (#2289)
  > Update patches in Ruby CI (#2233)
  > Documentation on service indicator (#2281)
  > Add the rehash utility to the openssl CLI tool (#2258)
  > Revert "Allow constructed strings in BER parsing (#2015)" (#2278)
  > Prepare AWS-LC v1.48.5 (#2274)
  > s2n-bignum build should use boringssl_prefix_symbols_asm.h (#2265)
  > ci: Nix flake and devShell (#2189)
  > GitHub CI job to verify tags are on expected branches (#2170)
  > Prepare for release v.1.48.4 (#2271)
  > Make AWS_LC_fips_failure_callback optional in builds with AWSLC_FIPS_FAILURE_CALLBACK (#2266)
  > Prepare v1.48.3 (#2269)
  > Update shard_gtest to unset environment variables once all the tests are done (#2270)
  > Minor build fixes for CMake and libssl on x86 (#2267)
  > Fix aws-lc-rs CI test (again) (#2268)
  > Add x4 batched SHAKE128 and SHAKE256 APIs (#2247)
  > Fix aws-lc-rs CI test when symbols removed (#2262)
  > Remove no-op register move from ChaCha20_ctr32_ssse3_4x (#2234)
  > Revert removal of "PEM_X509_INFO_write_bio" (#2226)
  > Use unsigned return type for BN_get_minimal_width and word size tests (#2260)
```
</details>

### Does this change impact existing behavior?

No.

### Does this change need a changelog entry? Does it require a version
change?

Yes. Updated as 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)](https://developercertificate.org/).

---------

Signed-off-by: Alessandro Passaro <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TingDaoK TingDaoK TingDaoK approved these changes

@DmitriyMusatkin DmitriyMusatkin DmitriyMusatkin approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@waahm7 @codecov-commenter @TingDaoK @DmitriyMusatkin