From caae0eba5ed1240e26f4c372588d9c24f0a81783 Mon Sep 17 00:00:00 2001 From: Dengke Date: Thu, 5 Sep 2024 11:31:32 -0700 Subject: [PATCH 1/3] use recv path from C directly --- runners/s3-benchrunner-c/CRunner.cpp | 32 +------------------ runners/s3-benchrunner-java/pom.xml | 2 +- .../s3benchrunner/crtjava/CRTJavaTask.java | 29 +---------------- 3 files changed, 3 insertions(+), 60 deletions(-) diff --git a/runners/s3-benchrunner-c/CRunner.cpp b/runners/s3-benchrunner-c/CRunner.cpp index aa3e00ad..13350f60 100644 --- a/runners/s3-benchrunner-c/CRunner.cpp +++ b/runners/s3-benchrunner-c/CRunner.cpp @@ -76,14 +76,6 @@ class Task promise donePromise; future doneFuture; - FILE *downloadFile = NULL; - - static int onDownloadData( - struct aws_s3_meta_request *meta_request, - const struct aws_byte_cursor *body, - uint64_t range_start, - void *user_data); - static void onFinished( struct aws_s3_meta_request *meta_request, const struct aws_s3_meta_request_result *meta_request_result, @@ -295,10 +287,7 @@ Task::Task(CRunner &runner, size_t taskI) if (runner.config.filesOnDisk) { - downloadFile = fopen(config.key.c_str(), "wb"); - AWS_FATAL_ASSERT(downloadFile != NULL); - - options.body_callback = Task::onDownloadData; + options.recv_filepath = toCursor(config.key); } } else @@ -367,29 +356,10 @@ void Task::onFinished( } // clean up task - if (task->downloadFile != NULL) - fclose(task->downloadFile); aws_s3_meta_request_release(task->metaRequest); task->donePromise.set_value(); } -int Task::onDownloadData( - struct aws_s3_meta_request *meta_request, - const struct aws_byte_cursor *body, - uint64_t range_start, - void *user_data) -{ - auto *task = static_cast(user_data); - - size_t written = fwrite(body->ptr, 1, body->len, task->downloadFile); - AWS_FATAL_ASSERT(written == body->len); - - // Increment read window so data will continue downloading - aws_s3_meta_request_increment_read_window(meta_request, body->len); - - return AWS_OP_SUCCESS; -} - int main(int argc, char *argv[]) { return benchmarkRunnerMain( diff --git a/runners/s3-benchrunner-java/pom.xml b/runners/s3-benchrunner-java/pom.xml index c402d768..c6234d67 100644 --- a/runners/s3-benchrunner-java/pom.xml +++ b/runners/s3-benchrunner-java/pom.xml @@ -10,7 +10,7 @@ - [0.30,) + [0.30.10,) [2.27,) 17 diff --git a/runners/s3-benchrunner-java/src/main/java/com/example/s3benchrunner/crtjava/CRTJavaTask.java b/runners/s3-benchrunner-java/src/main/java/com/example/s3benchrunner/crtjava/CRTJavaTask.java index 57d631bd..b0cd1ebf 100644 --- a/runners/s3-benchrunner-java/src/main/java/com/example/s3benchrunner/crtjava/CRTJavaTask.java +++ b/runners/s3-benchrunner-java/src/main/java/com/example/s3benchrunner/crtjava/CRTJavaTask.java @@ -10,12 +10,9 @@ import java.io.IOException; import java.nio.ByteBuffer; -import java.nio.channels.FileChannel; import java.nio.channels.ReadableByteChannel; -import java.nio.channels.WritableByteChannel; import java.nio.charset.StandardCharsets; import java.nio.file.Path; -import java.nio.file.StandardOpenOption; import java.util.ArrayList; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; @@ -28,7 +25,6 @@ class CRTJavaTask implements S3MetaRequestResponseHandler { S3MetaRequest metaRequest; CompletableFuture doneFuture; ReadableByteChannel uploadFileChannel; - WritableByteChannel downloadFileChannel; CRTJavaTask(CRTJavaBenchmarkRunner runner, int taskI) { this.runner = runner; @@ -66,12 +62,7 @@ class CRTJavaTask implements S3MetaRequestResponseHandler { headers.add(new HttpHeader("Content-Length", "0")); if (runner.config.filesOnDisk) { - try { - downloadFileChannel = FileChannel.open(Path.of(config.key), - StandardOpenOption.CREATE, StandardOpenOption.WRITE); - } catch (IOException e) { - throw new RuntimeException(e); - } + options.withResponseFilePath(Path.of(config.key)); } } else { throw new RuntimeException("Unknown task action: " + config.action); @@ -102,21 +93,6 @@ void waitUntilDone() { } } - @Override - public int onResponseBody(ByteBuffer bodyBytesIn, long objectRangeStart, long objectRangeEnd) { - int amountReceived = bodyBytesIn.remaining(); - - if (downloadFileChannel != null) { - try { - downloadFileChannel.write(bodyBytesIn); - } catch (IOException e) { - Util.exitWithError("Failed writing to file: " + e.toString()); - } - } - - return amountReceived; - } - @Override public void onFinished(S3FinishedResponseContext context) { if (context.getErrorCode() != 0) { @@ -136,9 +112,6 @@ public void onFinished(S3FinishedResponseContext context) { } else { // CRTJavaTask succeeded. Clean up... try { - if (downloadFileChannel != null) { - downloadFileChannel.close(); - } if (uploadFileChannel != null) { uploadFileChannel.close(); } From aa7adbb2847ae5d9c9803f28c1b0882fb05fe54f Mon Sep 17 00:00:00 2001 From: Dengke Date: Thu, 5 Sep 2024 11:35:28 -0700 Subject: [PATCH 2/3] disable backpressure for now. --- .../src/main/java/com/example/s3benchrunner/Main.java | 5 +++-- .../s3benchrunner/crtjava/CRTJavaBenchmarkRunner.java | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/runners/s3-benchrunner-java/src/main/java/com/example/s3benchrunner/Main.java b/runners/s3-benchrunner-java/src/main/java/com/example/s3benchrunner/Main.java index 86b9ff1b..b5bd69a8 100644 --- a/runners/s3-benchrunner-java/src/main/java/com/example/s3benchrunner/Main.java +++ b/runners/s3-benchrunner-java/src/main/java/com/example/s3benchrunner/Main.java @@ -11,8 +11,9 @@ public class Main { /////////////// BEGIN ARBITRARY HARDCODED VALUES /////////////// // 256MiB is Java Transfer Mgr v2 default. - // TODO: Investigate. At time of writing, this noticeably impacts performance. - public static final int BACKPRESSURE_INITIAL_READ_WINDOW_MiB = 256; + // This benchmark can turn off backpressure and rely solely on the + // memory-limiter. + public static final int BACKPRESSURE_INITIAL_READ_WINDOW_MiB = 0; /////////////// END ARBITRARY HARD-CODED VALUES /////////////// diff --git a/runners/s3-benchrunner-java/src/main/java/com/example/s3benchrunner/crtjava/CRTJavaBenchmarkRunner.java b/runners/s3-benchrunner-java/src/main/java/com/example/s3benchrunner/crtjava/CRTJavaBenchmarkRunner.java index bcaf6a50..a84f0c64 100644 --- a/runners/s3-benchrunner-java/src/main/java/com/example/s3benchrunner/crtjava/CRTJavaBenchmarkRunner.java +++ b/runners/s3-benchrunner-java/src/main/java/com/example/s3benchrunner/crtjava/CRTJavaBenchmarkRunner.java @@ -78,7 +78,7 @@ public CRTJavaBenchmarkRunner(BenchmarkConfig config, String bucket, String regi // If writing data to disk, enable backpressure. // This prevents us from running out of memory due to downloading // data faster than we can write it to disk. - if (config.filesOnDisk) { + if (config.filesOnDisk && Main.BACKPRESSURE_INITIAL_READ_WINDOW_MiB!=0) { s3ClientOpts.withReadBackpressureEnabled(true); s3ClientOpts.withInitialReadWindowSize(Util.bytesFromMiB(Main.BACKPRESSURE_INITIAL_READ_WINDOW_MiB)); } From de9c04b0a1dcced308121284e5f9b9ae4336fa81 Mon Sep 17 00:00:00 2001 From: Dengke Date: Thu, 5 Sep 2024 13:55:49 -0700 Subject: [PATCH 3/3] format --- .../s3benchrunner/crtjava/CRTJavaBenchmarkRunner.java | 2 +- .../example/s3benchrunner/crtjava/CRTJavaTask.java | 11 ----------- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/runners/s3-benchrunner-java/src/main/java/com/example/s3benchrunner/crtjava/CRTJavaBenchmarkRunner.java b/runners/s3-benchrunner-java/src/main/java/com/example/s3benchrunner/crtjava/CRTJavaBenchmarkRunner.java index a84f0c64..d5c2d020 100644 --- a/runners/s3-benchrunner-java/src/main/java/com/example/s3benchrunner/crtjava/CRTJavaBenchmarkRunner.java +++ b/runners/s3-benchrunner-java/src/main/java/com/example/s3benchrunner/crtjava/CRTJavaBenchmarkRunner.java @@ -78,7 +78,7 @@ public CRTJavaBenchmarkRunner(BenchmarkConfig config, String bucket, String regi // If writing data to disk, enable backpressure. // This prevents us from running out of memory due to downloading // data faster than we can write it to disk. - if (config.filesOnDisk && Main.BACKPRESSURE_INITIAL_READ_WINDOW_MiB!=0) { + if (config.filesOnDisk && Main.BACKPRESSURE_INITIAL_READ_WINDOW_MiB != 0) { s3ClientOpts.withReadBackpressureEnabled(true); s3ClientOpts.withInitialReadWindowSize(Util.bytesFromMiB(Main.BACKPRESSURE_INITIAL_READ_WINDOW_MiB)); } diff --git a/runners/s3-benchrunner-java/src/main/java/com/example/s3benchrunner/crtjava/CRTJavaTask.java b/runners/s3-benchrunner-java/src/main/java/com/example/s3benchrunner/crtjava/CRTJavaTask.java index b0cd1ebf..a0451a4b 100644 --- a/runners/s3-benchrunner-java/src/main/java/com/example/s3benchrunner/crtjava/CRTJavaTask.java +++ b/runners/s3-benchrunner-java/src/main/java/com/example/s3benchrunner/crtjava/CRTJavaTask.java @@ -8,9 +8,7 @@ import software.amazon.awssdk.crt.http.HttpRequestBodyStream; import software.amazon.awssdk.crt.s3.*; -import java.io.IOException; import java.nio.ByteBuffer; -import java.nio.channels.ReadableByteChannel; import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.util.ArrayList; @@ -24,7 +22,6 @@ class CRTJavaTask implements S3MetaRequestResponseHandler { TaskConfig config; S3MetaRequest metaRequest; CompletableFuture doneFuture; - ReadableByteChannel uploadFileChannel; CRTJavaTask(CRTJavaBenchmarkRunner runner, int taskI) { this.runner = runner; @@ -111,14 +108,6 @@ public void onFinished(S3FinishedResponseContext context) { Util.exitWithError("S3MetaRequest failed"); } else { // CRTJavaTask succeeded. Clean up... - try { - if (uploadFileChannel != null) { - uploadFileChannel.close(); - } - } catch (IOException e) { - Util.exitWithError("Failed closing file: " + e.toString()); - } - // work around API-gotcha where callbacks can fire on other threads // before makeMetaRequest() has returned synchronized (this) {