Skip to content

Commit 6a51a56

Browse files
ysaito1001rcoh
authored andcommitted
Create CredentialsCache and DefaultResolver only once in test/bench (#2620)
## Motivation and Context Addresses #2593 (comment) and #2593 (comment). Also ports the changes made in #2592. ## Description Prior to this PR, `sra_manual_test` and the bench `middleware_vs_orchestrator` created `CredentialsCache` and `aws_sdk_s3::endpoint::DefaultResolver` every time a request for `ListObjectsV2` was dispatched. This is not the case in production where those two types are created once during the construction of a service config ([here](https://github.com/awslabs/aws-sdk-rust/blob/main/sdk/s3/src/config.rs#L623-L625) and [here](https://github.com/awslabs/aws-sdk-rust/blob/main/sdk/s3/src/config.rs#L635-L652)) and reused for subsequent request dispatches. The PR will make `sra_manual_test` and `middleware_vs_orchestrator` do the same, creating `CredentialsCache` and `aws_sdk_s3::endpoint::DefaultResolver` only once before the test/benchmark starts running and reusing them thereafter. The change will help bring the performance for `orchestrator` closer to that for `middleware`. - In the `main` branch with `DefaultEndpointResolver` commented in and `StaticUriEndpointResolver` commented out: ``` middleware time: [20.924 µs 20.943 µs 20.964 µs] change: [-1.0107% -0.7357% -0.4827%] (p = 0.00 < 0.05) Change within noise threshold. Found 7 outliers among 100 measurements (7.00%) 1 (1.00%) high mild 6 (6.00%) high severe orchestrator time: [933.68 µs 940.11 µs 945.82 µs] change: [+2735.7% +2754.5% +2770.9%] (p = 0.00 < 0.05) Performance has regressed. Found 17 outliers among 100 measurements (17.00%) 14 (14.00%) low mild 2 (2.00%) high mild 1 (1.00%) high severe ``` - With the change in this PR: ``` middleware time: [21.161 µs 21.194 µs 21.232 µs] change: [-0.8973% -0.6397% -0.3758%] (p = 0.00 < 0.05) Change within noise threshold. Found 8 outliers among 100 measurements (8.00%) 5 (5.00%) high mild 3 (3.00%) high severe orchestrator time: [56.038 µs 56.182 µs 56.349 µs] change: [-0.7921% -0.5552% -0.3157%] (p = 0.00 < 0.05) Change within noise threshold. Found 5 outliers among 100 measurements (5.00%) 2 (2.00%) high mild 3 (3.00%) high severe ``` ## Testing Executed the following without any errors: ``` ➜ smithy-rs git:(ysaito/create-creds-cache-and-ep-resolver-only-once) ✗ ./gradlew aws:sra-test:assemble ➜ aws-sdk-s3 git:(ysaito/create-creds-cache-and-ep-resolver-only-once) cargo t ➜ aws-sdk-s3 git:(ysaito/create-creds-cache-and-ep-resolver-only-once) cargo bench ``` ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --------- Co-authored-by: Yuki Saito <awsaito@amazon.com>
1 parent b589bd0 commit 6a51a56

File tree

2 files changed

+77
-57
lines changed

2 files changed

+77
-57
lines changed

aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs

Lines changed: 42 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,13 @@
55

66
#[macro_use]
77
extern crate criterion;
8+
use aws_credential_types::cache::{CredentialsCache, SharedCredentialsCache};
9+
use aws_credential_types::provider::SharedCredentialsProvider;
10+
use aws_credential_types::Credentials;
811
use aws_sdk_s3 as s3;
912
use aws_smithy_client::erase::DynConnector;
1013
use aws_smithy_client::test_connection::infallible_connection_fn;
14+
use aws_smithy_http::endpoint::SharedEndpointResolver;
1115
use aws_smithy_runtime_api::type_erasure::TypedBox;
1216
use criterion::Criterion;
1317
use s3::operation::list_objects_v2::{ListObjectsV2Error, ListObjectsV2Input, ListObjectsV2Output};
@@ -22,10 +26,20 @@ async fn middleware(client: &s3::Client) {
2226
.expect("successful execution");
2327
}
2428

25-
async fn orchestrator(connector: &DynConnector) {
29+
async fn orchestrator(
30+
connector: &DynConnector,
31+
endpoint_resolver: SharedEndpointResolver<s3::endpoint::Params>,
32+
credentials_cache: SharedCredentialsCache,
33+
) {
34+
let service_runtime_plugin = orchestrator::ManualServiceRuntimePlugin {
35+
connector: connector.clone(),
36+
endpoint_resolver: endpoint_resolver.clone(),
37+
credentials_cache: credentials_cache.clone(),
38+
};
39+
2640
// TODO(enableNewSmithyRuntime): benchmark with `send_v2` directly once it works
2741
let runtime_plugins = aws_smithy_runtime_api::client::runtime_plugin::RuntimePlugins::new()
28-
.with_client_plugin(orchestrator::ManualServiceRuntimePlugin(connector.clone()))
42+
.with_client_plugin(service_runtime_plugin)
2943
.with_operation_plugin(aws_sdk_s3::operation::list_objects_v2::ListObjectsV2::new())
3044
.with_operation_plugin(orchestrator::ManualOperationRuntimePlugin);
3145
let input = ListObjectsV2Input::builder()
@@ -95,25 +109,32 @@ fn middleware_bench(c: &mut Criterion) {
95109

96110
fn orchestrator_bench(c: &mut Criterion) {
97111
let conn = test_connection();
112+
let endpoint_resolver = SharedEndpointResolver::new(s3::endpoint::DefaultResolver::new());
113+
let credentials_cache = SharedCredentialsCache::new(
114+
CredentialsCache::lazy()
115+
.create_cache(SharedCredentialsProvider::new(Credentials::for_tests())),
116+
);
98117

99118
c.bench_function("orchestrator", move |b| {
100119
b.to_async(tokio::runtime::Runtime::new().unwrap())
101-
.iter(|| async { orchestrator(&conn).await })
120+
.iter(|| async {
121+
orchestrator(&conn, endpoint_resolver.clone(), credentials_cache.clone()).await
122+
})
102123
});
103124
}
104125

105126
mod orchestrator {
106-
use aws_credential_types::cache::{CredentialsCache, SharedCredentialsCache};
107-
use aws_credential_types::provider::SharedCredentialsProvider;
108-
use aws_credential_types::Credentials;
127+
use aws_credential_types::cache::SharedCredentialsCache;
109128
use aws_http::user_agent::{ApiMetadata, AwsUserAgent};
110129
use aws_runtime::recursion_detection::RecursionDetectionInterceptor;
111130
use aws_runtime::user_agent::UserAgentInterceptor;
112131
use aws_sdk_s3::config::Region;
132+
use aws_sdk_s3::endpoint::Params;
113133
use aws_sdk_s3::operation::list_objects_v2::ListObjectsV2Input;
114134
use aws_smithy_client::erase::DynConnector;
135+
use aws_smithy_http::endpoint::SharedEndpointResolver;
115136
use aws_smithy_runtime::client::connections::adapter::DynConnectorAdapter;
116-
use aws_smithy_runtime_api::client::endpoints::StaticUriEndpointResolver;
137+
use aws_smithy_runtime::client::orchestrator::endpoints::DefaultEndpointResolver;
117138
use aws_smithy_runtime_api::client::interceptors::{
118139
Interceptor, InterceptorContext, InterceptorError, Interceptors,
119140
};
@@ -124,10 +145,13 @@ mod orchestrator {
124145
use aws_smithy_runtime_api::config_bag::ConfigBag;
125146
use aws_types::region::SigningRegion;
126147
use aws_types::SigningService;
127-
use http::Uri;
128148
use std::sync::Arc;
129149

130-
pub struct ManualServiceRuntimePlugin(pub DynConnector);
150+
pub struct ManualServiceRuntimePlugin {
151+
pub connector: DynConnector,
152+
pub endpoint_resolver: SharedEndpointResolver<Params>,
153+
pub credentials_cache: SharedCredentialsCache,
154+
}
131155

132156
impl RuntimePlugin for ManualServiceRuntimePlugin {
133157
fn configure(&self, cfg: &mut ConfigBag) -> Result<(), BoxError> {
@@ -136,9 +160,7 @@ mod orchestrator {
136160
.identity_resolver(
137161
aws_runtime::auth::sigv4::SCHEME_ID,
138162
aws_runtime::identity::credentials::CredentialsIdentityResolver::new(
139-
SharedCredentialsCache::new(CredentialsCache::lazy().create_cache(
140-
SharedCredentialsProvider::new(Credentials::for_tests()),
141-
)),
163+
self.credentials_cache.clone(),
142164
),
143165
)
144166
.identity_resolver(
@@ -163,14 +185,7 @@ mod orchestrator {
163185
),
164186
);
165187

166-
//cfg.set_endpoint_resolver(DefaultEndpointResolver::new(
167-
// aws_smithy_http::endpoint::SharedEndpointResolver::new(
168-
// aws_sdk_s3::endpoint::DefaultResolver::new(),
169-
// ),
170-
//));
171-
cfg.set_endpoint_resolver(StaticUriEndpointResolver::uri(Uri::from_static(
172-
"https://test-bucket.s3.us-east-1.amazonaws.com/",
173-
)));
188+
cfg.set_endpoint_resolver(DefaultEndpointResolver::new(self.endpoint_resolver.clone()));
174189

175190
let params_builder = aws_sdk_s3::endpoint::Params::builder()
176191
.set_region(Some("us-east-1".to_owned()))
@@ -182,7 +197,7 @@ mod orchestrator {
182197
);
183198

184199
let connection: Box<dyn Connection> =
185-
Box::new(DynConnectorAdapter::new(self.0.clone()));
200+
Box::new(DynConnectorAdapter::new(self.connector.clone()));
186201
cfg.set_connection(connection);
187202

188203
cfg.set_trace_probe({
@@ -227,12 +242,10 @@ mod orchestrator {
227242
let input = context.input()?;
228243
let input = input
229244
.downcast_ref::<ListObjectsV2Input>()
230-
.ok_or_else(|| InterceptorError::invalid_input_access())?;
245+
.ok_or_else(|| "failed to downcast to ListObjectsV2Input")?;
231246
let mut params_builder = cfg
232247
.get::<aws_sdk_s3::endpoint::ParamsBuilder>()
233-
.ok_or(InterceptorError::read_before_execution(
234-
"missing endpoint params builder",
235-
))?
248+
.ok_or_else(|| "missing endpoint params builder")?
236249
.clone();
237250
params_builder = params_builder.set_bucket(input.bucket.clone());
238251
cfg.put(params_builder);
@@ -251,13 +264,11 @@ mod orchestrator {
251264
) -> Result<(), BoxError> {
252265
let params_builder = cfg
253266
.get::<aws_sdk_s3::endpoint::ParamsBuilder>()
254-
.ok_or(InterceptorError::read_before_execution(
255-
"missing endpoint params builder",
256-
))?
267+
.ok_or_else(|| "missing endpoint params builder")?
257268
.clone();
258-
let params = params_builder
259-
.build()
260-
.map_err(InterceptorError::read_before_execution)?;
269+
let params = params_builder.build().map_err(|err| {
270+
ContextAttachedError::new("endpoint params could not be built", err)
271+
})?;
261272
cfg.put(
262273
aws_smithy_runtime_api::client::orchestrator::EndpointResolverParams::new(
263274
params,

aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs

Lines changed: 35 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,18 @@ use aws_runtime::auth::sigv4::SigV4OperationSigningConfig;
1010
use aws_runtime::recursion_detection::RecursionDetectionInterceptor;
1111
use aws_runtime::user_agent::UserAgentInterceptor;
1212
use aws_sdk_s3::config::{Credentials, Region};
13+
use aws_sdk_s3::endpoint::Params;
1314
use aws_sdk_s3::operation::list_objects_v2::{
1415
ListObjectsV2Error, ListObjectsV2Input, ListObjectsV2Output,
1516
};
1617
use aws_sdk_s3::primitives::SdkBody;
1718
use aws_smithy_client::erase::DynConnector;
1819
use aws_smithy_client::test_connection::TestConnection;
20+
use aws_smithy_http::endpoint::SharedEndpointResolver;
1921
use aws_smithy_runtime::client::connections::adapter::DynConnectorAdapter;
2022
use aws_smithy_runtime::client::orchestrator::endpoints::DefaultEndpointResolver;
21-
use aws_smithy_runtime_api::client::interceptors::{
22-
Interceptor, InterceptorContext, InterceptorError, Interceptors,
23-
};
23+
use aws_smithy_runtime_api::client::interceptors::error::ContextAttachedError;
24+
use aws_smithy_runtime_api::client::interceptors::{Interceptor, InterceptorContext, Interceptors};
2425
use aws_smithy_runtime_api::client::orchestrator::{
2526
BoxError, ConfigBagAccessors, Connection, HttpRequest, HttpResponse, TraceProbe,
2627
};
@@ -74,7 +75,11 @@ async fn sra_test() {
7475
async fn sra_manual_test() {
7576
tracing_subscriber::fmt::init();
7677

77-
struct ManualServiceRuntimePlugin(TestConnection<&'static str>);
78+
struct ManualServiceRuntimePlugin {
79+
connector: TestConnection<&'static str>,
80+
endpoint_resolver: SharedEndpointResolver<Params>,
81+
credentials_cache: SharedCredentialsCache,
82+
}
7883

7984
impl RuntimePlugin for ManualServiceRuntimePlugin {
8085
fn configure(&self, cfg: &mut ConfigBag) -> Result<(), BoxError> {
@@ -83,9 +88,7 @@ async fn sra_manual_test() {
8388
.identity_resolver(
8489
aws_runtime::auth::sigv4::SCHEME_ID,
8590
aws_runtime::identity::credentials::CredentialsIdentityResolver::new(
86-
SharedCredentialsCache::new(CredentialsCache::lazy().create_cache(
87-
SharedCredentialsProvider::new(Credentials::for_tests()),
88-
)),
91+
self.credentials_cache.clone(),
8992
),
9093
)
9194
.identity_resolver(
@@ -110,13 +113,9 @@ async fn sra_manual_test() {
110113
),
111114
);
112115

113-
cfg.set_endpoint_resolver(DefaultEndpointResolver::new(
114-
aws_smithy_http::endpoint::SharedEndpointResolver::new(
115-
aws_sdk_s3::endpoint::DefaultResolver::new(),
116-
),
117-
));
116+
cfg.set_endpoint_resolver(DefaultEndpointResolver::new(self.endpoint_resolver.clone()));
118117

119-
let params_builder = aws_sdk_s3::endpoint::Params::builder()
118+
let params_builder = Params::builder()
120119
.set_region(Some("us-east-1".to_owned()))
121120
.set_endpoint(Some("https://s3.us-east-1.amazonaws.com/".to_owned()));
122121
cfg.put(params_builder);
@@ -125,8 +124,9 @@ async fn sra_manual_test() {
125124
aws_smithy_runtime_api::client::retries::NeverRetryStrategy::new(),
126125
);
127126

128-
let connection: Box<dyn Connection> =
129-
Box::new(DynConnectorAdapter::new(DynConnector::new(self.0.clone())));
127+
let connection: Box<dyn Connection> = Box::new(DynConnectorAdapter::new(
128+
DynConnector::new(self.connector.clone()),
129+
));
130130
cfg.set_connection(connection);
131131

132132
cfg.set_trace_probe({
@@ -189,12 +189,10 @@ async fn sra_manual_test() {
189189
let input = context.input()?;
190190
let input = input
191191
.downcast_ref::<ListObjectsV2Input>()
192-
.ok_or_else(|| InterceptorError::invalid_input_access())?;
192+
.ok_or_else(|| "failed to downcast to ListObjectsV2Input")?;
193193
let mut params_builder = cfg
194194
.get::<aws_sdk_s3::endpoint::ParamsBuilder>()
195-
.ok_or(InterceptorError::read_before_execution(
196-
"missing endpoint params builder",
197-
))?
195+
.ok_or_else(|| "missing endpoint params builder")?
198196
.clone();
199197
params_builder = params_builder.set_bucket(input.bucket.clone());
200198
cfg.put(params_builder);
@@ -213,13 +211,11 @@ async fn sra_manual_test() {
213211
) -> Result<(), BoxError> {
214212
let params_builder = cfg
215213
.get::<aws_sdk_s3::endpoint::ParamsBuilder>()
216-
.ok_or(InterceptorError::read_before_execution(
217-
"missing endpoint params builder",
218-
))?
214+
.ok_or_else(|| "missing endpoint params builder")?
219215
.clone();
220-
let params = params_builder
221-
.build()
222-
.map_err(InterceptorError::read_before_execution)?;
216+
let params = params_builder.build().map_err(|err| {
217+
ContextAttachedError::new("endpoint params could not be built", err)
218+
})?;
223219
cfg.put(
224220
aws_smithy_runtime_api::client::orchestrator::EndpointResolverParams::new(
225221
params,
@@ -265,8 +261,21 @@ async fn sra_manual_test() {
265261
"#).unwrap(),
266262
)]);
267263

264+
let endpoint_resolver =
265+
SharedEndpointResolver::new(aws_sdk_s3::endpoint::DefaultResolver::new());
266+
let credentials_cache = SharedCredentialsCache::new(
267+
CredentialsCache::lazy()
268+
.create_cache(SharedCredentialsProvider::new(Credentials::for_tests())),
269+
);
270+
271+
let service_runtime_plugin = ManualServiceRuntimePlugin {
272+
connector: conn.clone(),
273+
endpoint_resolver,
274+
credentials_cache,
275+
};
276+
268277
let runtime_plugins = aws_smithy_runtime_api::client::runtime_plugin::RuntimePlugins::new()
269-
.with_client_plugin(ManualServiceRuntimePlugin(conn.clone()))
278+
.with_client_plugin(service_runtime_plugin)
270279
.with_operation_plugin(aws_sdk_s3::operation::list_objects_v2::ListObjectsV2::new())
271280
.with_operation_plugin(ManualOperationRuntimePlugin);
272281

0 commit comments

Comments
 (0)