-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
use stream_copy_to_stream #1014
Conversation
it's simpler, and should be faster. For example, stream_copy_to_stream can use sendfile ( https://man7.org/linux/man-pages/man2/sendfile.2.html ) on operating systems supporting it, which is faster and use less RAM than fread()+fwrite() (because it avoids copying data to/from userland, doing the copy entirely in-kernel~)
This might be a problem under 32bit environments with files bigger than 4GiB (bug #74395). At best if (PHP_INT_SIZE === 4 || (strtoupper(substr(PHP_OS, 0, 3)) === 'WIN' && version_compare(phpversion(), '8.0.9', '<'))) {
while ($buff = fread($in, 4096)) { fwrite($out, $buff); }
} else {
stream_copy_to_stream($in, $out);
} |
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.
@ner00 even just testing 32bit php5.5 is very cubersome; i installed a 32bit debian 11 VM, couldn't find a php5.5 pre-built binary, tried to compile php5.5, but building php5.5 from source requires an ancient 2012 version of GNU Bison (2.7), couldn't find a pre-built bison2.7, trying to build bison2.7 from source ends with this error:
googlging that lead me to this patch: https://github.com/rdslw/openwrt/blob/e5d47f32131849a69a9267de51a30d6be1f0d0ac/tools/bison/patches/110-glibc-change-work-around.patch after applying that patch to bison, i was finally able to get bison2.7 to build with
, and finally able to build PHP5.5 with
and voila
and now to test stream_copy_to_stream on a 5GB file:
well.... that was certainly an unexpected result, but the problem isn't stream_copy_to_stream, it's fopen! :O (at least on 32bit php5.5.38) |
trying the same on a file that is exactly 0xFFFFFFFF ( 4294967295 ) bytes, aka the max possible value of a uint32_t (keep in mind, 32bit php's int is int32_t, not uint32_t) yield the same fopen error. trying on a file that is exactly 0x7FFFFFFF (2147483647 ) bytes, aka 32bit PHP's PHP_INT_MAX yields:
success! didn't hit any stream_copy_to_stream bug, when using the oldest 32bit php supported by tinyfilemanager. trying 2147483648 bytes (PHP_INT_MAX + 1) yielded the same fopen error. |
to keep it simple, how about we just do if (PHP_VERSION_ID < 80009) {
// workaround https://bugs.php.net/bug.php?id=81145
while ($buff = fread($in, 4096)) { fwrite($out, $buff); }
} else {
stream_copy_to_stream($in, $out);
} ? |
ouch shit, just found a bug in the original code: https://3v4l.org/PItRM while (!in_array($buff = fread($in, 4096), array("", false), true)) { fwrite($out, $buff); } otherwise it will fail on files being any multiple of 4096-bytes+1 and ending with |
PR updated, fixed the while loop early return, and only use stream_copy_to_stream on php >= 8.0.9 because of https://bugs.php.net/bug.php?id=81145 must admit, if you're into micro-optimizations, doing for(;;){
$buff = fread($in, 4096);
if($buff === false || $buff === ""){
break;
}
fwrite($out, $buff);
} would use fewer cpu cycles, avoiding a relatively expensive function call (in_array).. but meh, bet you'd have to use hyperfine to heven notice a difference. |
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.
Looks good to me.
Seems to have an average performance increase anywhere between 15% to 30% using stream_copy_to_stream
@divinity76 and @ner00 thanks for your support, I'll verify and merge sometime soon. |
when this is merged, it will also fix #1016 :) |
* use stream_copy_to_stream it's simpler, and should be faster. For example, stream_copy_to_stream can use sendfile ( https://man7.org/linux/man-pages/man2/sendfile.2.html ) on operating systems supporting it, which is faster and use less RAM than fread()+fwrite() (because it avoids copying data to/from userland, doing the copy entirely in-kernel~) * fix loop early return, and workaround bug * use feof ref prasathmani#1016 (comment)
* use stream_copy_to_stream it's simpler, and should be faster. For example, stream_copy_to_stream can use sendfile ( https://man7.org/linux/man-pages/man2/sendfile.2.html ) on operating systems supporting it, which is faster and use less RAM than fread()+fwrite() (because it avoids copying data to/from userland, doing the copy entirely in-kernel~) * fix loop early return, and workaround bug * use feof ref prasathmani#1016 (comment)
* use stream_copy_to_stream it's simpler, and should be faster. For example, stream_copy_to_stream can use sendfile ( https://man7.org/linux/man-pages/man2/sendfile.2.html ) on operating systems supporting it, which is faster and use less RAM than fread()+fwrite() (because it avoids copying data to/from userland, doing the copy entirely in-kernel~) * fix loop early return, and workaround bug * use feof ref prasathmani#1016 (comment)
* use stream_copy_to_stream it's simpler, and should be faster. For example, stream_copy_to_stream can use sendfile ( https://man7.org/linux/man-pages/man2/sendfile.2.html ) on operating systems supporting it, which is faster and use less RAM than fread()+fwrite() (because it avoids copying data to/from userland, doing the copy entirely in-kernel~) * fix loop early return, and workaround bug * use feof ref prasathmani#1016 (comment)
* use stream_copy_to_stream it's simpler, and should be faster. For example, stream_copy_to_stream can use sendfile ( https://man7.org/linux/man-pages/man2/sendfile.2.html ) on operating systems supporting it, which is faster and use less RAM than fread()+fwrite() (because it avoids copying data to/from userland, doing the copy entirely in-kernel~) * fix loop early return, and workaround bug * use feof ref prasathmani#1016 (comment)
* use stream_copy_to_stream it's simpler, and should be faster. For example, stream_copy_to_stream can use sendfile ( https://man7.org/linux/man-pages/man2/sendfile.2.html ) on operating systems supporting it, which is faster and use less RAM than fread()+fwrite() (because it avoids copying data to/from userland, doing the copy entirely in-kernel~) * fix loop early return, and workaround bug * use feof ref prasathmani#1016 (comment)
* use stream_copy_to_stream it's simpler, and should be faster. For example, stream_copy_to_stream can use sendfile ( https://man7.org/linux/man-pages/man2/sendfile.2.html ) on operating systems supporting it, which is faster and use less RAM than fread()+fwrite() (because it avoids copying data to/from userland, doing the copy entirely in-kernel~) * fix loop early return, and workaround bug * use feof ref prasathmani#1016 (comment)
* publish additional docker tags (#975) * Update Romanian translations (#981) * Update tinyfilemanager.php * Prevent logout issue after page was cached (#1004) Logout may not work otherwise, browser reloads cached page from disk instead of sending GET request ?logout=1 to server. * tell git to always commit .php in unix-newlines (#1017) so hopefully we don't get a repeat of #994 (comment) * Check if posix_getpwuid/posix_getgrgid calls were successful (#1023) * use stream_copy_to_stream (#1014) * use stream_copy_to_stream it's simpler, and should be faster. For example, stream_copy_to_stream can use sendfile ( https://man7.org/linux/man-pages/man2/sendfile.2.html ) on operating systems supporting it, which is faster and use less RAM than fread()+fwrite() (because it avoids copying data to/from userland, doing the copy entirely in-kernel~) * fix loop early return, and workaround bug * use feof ref #1016 (comment) * added bengali translation (#1018) * Fix upload of existing files (#1026) * Fix typo. (#1028) * login (Redirecting to Main domain of website instead of tfm.php) fix (#1031) When logged in it takes to the website's main URL. For example, if I have tfm in www.example.com/tfm/index.php (index.php is tfm) then after logging in it redirects to www.example.com and then have to press back on the browser then it takes to www.example.com/tfm/index.php * Add configurable path display modes for better privacy and clarity (#1034) * Resize preview image and implement zoom in/out (#1036) * Resize preview image and implement zoom in/out * Remove redundant class name --------- Co-authored-by: ssams <[email protected]> Co-authored-by: Sergiu Bivol <[email protected]> Co-authored-by: Prasath Mani <[email protected]> Co-authored-by: divinity76 <[email protected]> Co-authored-by: Micha Ober <[email protected]> Co-authored-by: Joy Biswas <[email protected]> Co-authored-by: Micha Ober <[email protected]> Co-authored-by: Caleb Mazalevskis <[email protected]> Co-authored-by: xololunatic <[email protected]> Co-authored-by: DannyDaemonic <[email protected]>
it's simpler, and should be faster.
For example, stream_copy_to_stream can use sendfile ( https://man7.org/linux/man-pages/man2/sendfile.2.html ) on operating systems supporting it, which is faster and use less RAM than fread()+fwrite() (because it avoids copying data to/from userland, doing the copy entirely in-kernel~)