Skip to content

Commit 0395c14

Browse files
committed
address code review feedback: refine CMakeLists.txt, replace std::set with std::array, optimize StringStream construction, add client null checks, and prevent possible ODR violations by moving config definitions to .cpp files.
1 parent dfbcdf2 commit 0395c14

File tree

5 files changed

+59
-25
lines changed

5 files changed

+59
-25
lines changed

tests/performance-tests/CMakeLists.txt

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,21 @@ FetchContent_Declare(
1717
)
1818
FetchContent_MakeAvailable(cxxopts)
1919

20-
function(add_service_test SERVICE SDK_LIB PERF_TEST_FILE)
21-
add_executable(${SERVICE}-performance-test
22-
src/services/${SERVICE}/main.cpp
20+
function(add_service_test SERVICE)
21+
string(TOLOWER ${SERVICE} SERVICE_LOWER)
22+
add_executable(${SERVICE_LOWER}-performance-test
23+
src/services/${SERVICE_LOWER}/main.cpp
2324
src/reporting/JsonReportingMetrics.cpp
24-
src/services/${SERVICE}/${PERF_TEST_FILE}
25+
src/services/${SERVICE_LOWER}/${SERVICE}PerformanceTest.cpp
26+
src/services/${SERVICE_LOWER}/${SERVICE}TestConfig.cpp
2527
)
26-
set_compiler_flags(${SERVICE}-performance-test)
27-
set_compiler_warnings(${SERVICE}-performance-test)
28-
target_include_directories(${SERVICE}-performance-test PRIVATE include)
29-
target_link_libraries(${SERVICE}-performance-test PRIVATE aws-cpp-sdk-core ${SDK_LIB} cxxopts::cxxopts)
30-
target_compile_options(${SERVICE}-performance-test PRIVATE -std=c++17 -fexceptions)
28+
set_compiler_flags(${SERVICE_LOWER}-performance-test)
29+
set_compiler_warnings(${SERVICE_LOWER}-performance-test)
30+
target_include_directories(${SERVICE_LOWER}-performance-test PRIVATE include)
31+
target_link_libraries(${SERVICE_LOWER}-performance-test PRIVATE aws-cpp-sdk-core aws-cpp-sdk-${SERVICE_LOWER} cxxopts::cxxopts)
32+
set_property(TARGET ${SERVICE_LOWER}-performance-test PROPERTY CXX_STANDARD 20)
33+
set_property(TARGET ${SERVICE_LOWER}-performance-test PROPERTY COMPILE_OPTIONS "-fexceptions")
3134
endfunction()
3235

33-
add_service_test(s3 aws-cpp-sdk-s3 S3PerformanceTest.cpp)
34-
add_service_test(dynamodb aws-cpp-sdk-dynamodb DynamoDBPerformanceTest.cpp)
36+
add_service_test(S3)
37+
add_service_test(DynamoDB)

tests/performance-tests/include/performance-tests/services/s3/S3PerformanceTest.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ struct TestCase {
3030
*/
3131
class S3PerformanceTest : public PerformanceTestBase {
3232
public:
33-
S3PerformanceTest(const Aws::String& region, const TestCase& config, const Aws::String& availabilityZoneId, int iterations = 3);
33+
S3PerformanceTest(const Aws::String& region, const TestCase& config, const Aws::String& availabilityZoneId, int iterations = 10);
3434

3535
void Setup() override;
3636
void TearDown() override;

tests/performance-tests/include/performance-tests/services/s3/S3TestConfig.h

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,14 @@
88
#include <performance-tests/services/s3/S3PerformanceTest.h>
99

1010
#include <array>
11-
#include <set>
1211

1312
namespace PerformanceTest {
1413
namespace Services {
1514
namespace S3 {
1615
namespace TestConfig {
17-
const std::set<const char*> TestOperations = {"PutObject", "GetObject"};
18-
19-
const std::array<TestCase, 6> TestMatrix = {{{"8KB", 8 * 1024, "s3-standard"},
20-
{"64KB", 64 * 1024, "s3-standard"},
21-
{"1MB", 1024 * 1024, "s3-standard"},
22-
{"8KB", 8 * 1024, "s3-express"},
23-
{"64KB", 64 * 1024, "s3-express"},
24-
{"1MB", 1024 * 1024, "s3-express"}}};
25-
26-
const char* OutputFilename = "s3-performance-test-results.json";
16+
extern const std::array<const char*, 2> TestOperations;
17+
extern const std::array<TestCase, 6> TestMatrix;
18+
extern const char* OutputFilename;
2719
} // namespace TestConfig
2820
} // namespace S3
2921
} // namespace Services

tests/performance-tests/src/services/s3/S3PerformanceTest.cpp

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,11 @@ void PerformanceTest::Services::S3::S3PerformanceTest::Setup() {
6363
}
6464

6565
void PerformanceTest::Services::S3::S3PerformanceTest::Run() {
66+
if (!m_s3) {
67+
AWS_LOG_ERROR("PerformanceTest", "S3:Run - S3 client not initialized, Setup() failed or was not called");
68+
return;
69+
}
70+
6671
if (m_bucketName.empty()) {
6772
AWS_LOG_ERROR("PerformanceTest", "S3:Run - Bucket setup failed, skipping test");
6873
return;
@@ -72,8 +77,7 @@ void PerformanceTest::Services::S3::S3PerformanceTest::Run() {
7277

7378
// Run PutObject multiple times
7479
for (int i = 0; i < m_iterations; i++) {
75-
auto stream = Aws::MakeShared<Aws::StringStream>("PerfStream");
76-
*stream << randomPayload;
80+
const auto stream = Aws::MakeShared<Aws::StringStream>("PerfStream", randomPayload);
7781

7882
Aws::S3::Model::PutObjectRequest por;
7983
por.WithBucket(m_bucketName).WithKey("test-object-" + Aws::Utils::StringUtils::to_string(i)).SetBody(stream);
@@ -99,6 +103,11 @@ void PerformanceTest::Services::S3::S3PerformanceTest::Run() {
99103
}
100104

101105
void PerformanceTest::Services::S3::S3PerformanceTest::TearDown() {
106+
if (!m_s3) {
107+
AWS_LOG_ERROR("PerformanceTest", "S3:TearDown - S3 client not initialized, Setup() failed or was not called");
108+
return;
109+
}
110+
102111
if (m_bucketName.empty()) {
103112
AWS_LOG_ERROR("PerformanceTest", "S3:TearDown - No bucket to clean up, setup likely failed");
104113
return;
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/**
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0.
4+
*/
5+
6+
#include <performance-tests/services/s3/S3PerformanceTest.h>
7+
#include <performance-tests/services/s3/S3TestConfig.h>
8+
9+
#include <array>
10+
11+
namespace PerformanceTest {
12+
namespace Services {
13+
namespace S3 {
14+
namespace TestConfig {
15+
16+
const std::array<const char*, 2> TestOperations = {"PutObject", "GetObject"};
17+
18+
const std::array<TestCase, 6> TestMatrix = {{{.sizeLabel = "8KB", .sizeBytes = 8 * 1024, .bucketTypeLabel = "s3-standard"},
19+
{.sizeLabel = "64KB", .sizeBytes = 64 * 1024, .bucketTypeLabel = "s3-standard"},
20+
{.sizeLabel = "1MB", .sizeBytes = 1024 * 1024, .bucketTypeLabel = "s3-standard"},
21+
{.sizeLabel = "8KB", .sizeBytes = 8 * 1024, .bucketTypeLabel = "s3-express"},
22+
{.sizeLabel = "64KB", .sizeBytes = 64 * 1024, .bucketTypeLabel = "s3-express"},
23+
{.sizeLabel = "1MB", .sizeBytes = 1024 * 1024, .bucketTypeLabel = "s3-express"}}};
24+
25+
const char* OutputFilename = "s3-performance-test-results.json";
26+
27+
} // namespace TestConfig
28+
} // namespace S3
29+
} // namespace Services
30+
} // namespace PerformanceTest

0 commit comments

Comments
 (0)