Skip to content
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.

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

Optimize performance for S3Escaper.encode #1528

Closed
kzhsw opened this issue Feb 2, 2024 · 4 comments · Fixed by #1529
Closed

Optimize performance for S3Escaper.encode #1528

kzhsw opened this issue Feb 2, 2024 · 4 comments · Fixed by #1529

Comments

@kzhsw
Copy link
Contributor

kzhsw commented Feb 2, 2024

I'm building some minio-based java app, and found out S3Escaper.encode and the String.replaceAll call inside it is one of the bottlenecks of the whole app after some profiling.
The optimized code is here.
The JMH benchmark result on github actions ci is here

Benchmark                                    Mode  Cnt          Score          Error  Units
S3EscaperBenchmark.benchmarkEncode           avgt   10  106580200.431 ± 39231338.664  ns/op
S3EscaperBenchmark.benchmarkEncodeOptimized  avgt   10    6190071.878 ±    22742.019  ns/op
@balamurugana
Copy link
Member

I wonder how this method affects overall performance including network/disk I/O. Please share your sample code here.

@kzhsw
Copy link
Contributor Author

kzhsw commented Feb 2, 2024

I'm using MinioClient.getPresignedObjectUrl for making presigned url for downloading some small images, so there would be many requests.

final GetPresignedObjectUrlArgs.Builder builder =
        GetPresignedObjectUrlArgs.builder();

if (StringUtils.isNotEmpty(file.getRegionName())) {
    builder.region(file.getRegionName());
}
builder.bucket(file.getBucketName());
builder.object(file.getObjectPath());
if (StringUtils.isNotEmpty(file.getFileName())) {
    builder.extraQueryParams(Collections.singletonMap(
            "response-content-disposition",
            contentDisposition(file.getFileName(), attachment).toString()
    ));
}
final GetPresignedObjectUrlArgs args = builder
        .method(Method.GET)
        .expiry(Math.toIntExact(expireAfter.toMillis() / 1000))
        .build();

return client.getPresignedObjectUrl(args);

the flame graph
the call tree

@balamurugana
Copy link
Member

Make sense. Please send a PR.

kzhsw added a commit to kzhsw/minio-java that referenced this issue Feb 3, 2024
Replace `String.replaceAll` with StringBuilder for better performance.
See also benchmark at <https://github.com/kzhsw/minio-java-jmh>.
Close minio#1528
@kzhsw
Copy link
Contributor Author

kzhsw commented Feb 3, 2024

Make sense. Please send a PR.

It's here: #1529

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants