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 GridFS throughput by removing redundant byte array cloning. #1402

Merged
merged 5 commits into from
Jun 5, 2024

Conversation

vbabanin
Copy link
Member

@vbabanin vbabanin commented May 28, 2024

Description

This PR introduces performance improvement in the GridFS implementation of the sync Java driver, specifically targeting the GridFSDownloadStream and GridFSUploadStream classes. It was identified that byte arrays are cloned twice internally via the Binary class, which impacts throughput.

Key Changes:

Modified GridFSDownloadStream and GridFSUploadStream to use BsonDocument instead of Document. The BsonDocument implementation avoids the unnecessary copying in the BsonBinary class, thus enhancing performance.

Performance Improvements:

GridFS Download:
Stable Region Mean on main branch = 769.21 MBps
Perf result mean = 1030.63 MBps
Improvement: Approximately 34.02% increase in throughput

GridFS Upload:
Stable Region Mean on main branch = 450.62 MBps
Perf result mean = 515.84 MBps
Improvement: Approximately 14.48% increase in throughput

Additional Information:

For further details and results of performance tests executed on this patch, please refer to the performance test results.

JAVA-5485

Copy link
Collaborator

@jyemin jyemin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code style suggestions. Otherwise LGTM assuming performance is improved.

@vbabanin vbabanin self-assigned this Jun 4, 2024
@vbabanin vbabanin requested review from rozza and jyemin June 4, 2024 13:16
@vbabanin vbabanin marked this pull request as ready for review June 4, 2024 13:16
@rozza
Copy link
Member

rozza commented Jun 5, 2024

LGTM - great find and fix, quite a subtle change / fix.

@vbabanin vbabanin merged commit da30e52 into mongodb:master Jun 5, 2024
52 of 60 checks passed
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 this pull request may close these issues.

3 participants