-
Notifications
You must be signed in to change notification settings - Fork 263
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
PHPLIB-1237 Implement Parallel GridFS multi-file upload & download benchmarks #1170
Conversation
|
||
private static function getFileNames(): array | ||
{ | ||
$tempDir = sys_get_temp_dir() . '/mongodb-php-benchmark'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is the same temp directory referenced in a previous PR. Would it make sense to use a constant or environment variable for this? I assume phpbench may have something like phpunit.xml
for specifying env vars.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tmp directory is a system setting. It can be changed with sys_temp_dir
ini setting or with the TMPDIR
env var.
I don't think anyone will ever need to change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was mostly referring to mongodb-php-benchmark
since it's repeated across tests. No worries.
$stream = Utils::getDatabase() | ||
->selectGridFSBucket() | ||
->openDownloadStreamByName(basename($file)); | ||
stream_copy_to_stream($stream, fopen($file, 'w')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was there a particular reason you didn't use Bucket::downloadToStream()
here?
I wouldn't expect a performance difference since that method basically calls stream_copy_to_stream()
. Just curious.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No particular reason. Actually I missed downloadToStreamByName
.
I'm not sure we need to switch the implement for this benchmark.
Fix PHPLIB-1237
Follows #1166
Parallel Benchmarks specs: GridFS upload & download
Single implementation using one fork for each file uploaded/downloaded.