Fix bug #63937: Upload speed 10 times slower with PHP #13041
Conversation
There are two slow parts in the upload logic: - Reading from the input stream character by character - Checking each character one by one to normalize line endings First of all, the line normalization isn't necessary for binary transfers, so we can use a simple read while loop to read bytes into the transfer buffer. Second, for the ASCII transfer where we do have to normalize line endings, we can be smarter than reading one character at a time. There's a php_stream_get_line() function that we can repurpose if the flags for the stream are set up properly. This patch implements these fixes. Results: I tested this on an 850 MiB file, transferring this to an FTP server running locally. Results before patch: Binary/ASCII transfer (same code path): 8.21s Results after patch: Binary transfer: 0.65s ASCII transfer: 0.74s Further improvement is probably possible by having a larger send buffer.
|
This has been laying around for quite some time and CI looks happy. Is there something else need to be done to merge? |
There was a problem hiding this comment.
Nice work! I wouldn't actually expect such a big difference as I would expect php_stream_getc to read from stream read buffer (chunk size) which it should. I actually look through the code and can't see why this is so much faster. But I guess for big uploads all those things add up.
In any case it's for sure a good improvement. Think it's more a feature so I would probably go just for master only.
|
Actually maybe that setting ptr byte by byte might be that part that makes most difference. Anyway doesn't matter that much. As I said it's a nice improvement in any case. |
This is going by memory, but I did look at the assembly, and it's just the overhead of doing a chain of calls for only retrieving one byte at a time, whereas the EOL detection stays within the same function and uses memchr which does SIMD.
Agreed. |
Reorganize performance section. Add performance entry for GH-13041.
There are two slow parts in the upload logic:
First of all, the line normalization isn't necessary for binary transfers, so we can use a simple read while loop to read bytes into the transfer buffer.
Second, for the ASCII transfer where we do have to normalize line endings, we can be smarter than reading one character at a time. There's a php_stream_get_line() function that we can repurpose if the flags for the stream are set up properly.
This patch implements these fixes.
Results: I tested this on an 850 MiB file, transferring this to an FTP server running locally.
Results before patch:
Binary/ASCII transfer (same code path): 8.21s
Results after patch:
Binary transfer: 0.65s
ASCII transfer: 0.74s
Further improvement is probably possible by having a larger send buffer.