fix: Avoid extra stream copy to memory/temp if we can determine the size#38407
fix: Avoid extra stream copy to memory/temp if we can determine the size#38407juliusknorr wants to merge 2 commits intomasterfrom
Conversation
Signed-off-by: Julius Härtl <[email protected]>
Signed-off-by: Julius Härtl <[email protected]>
juliusknorr
force-pushed
the
bugfix/avoid-extra-stream-copy
branch
from
0277e70 to
a2b37a8
Compare
juliusknorr
changed the title
juliusknorr
added
3. to review
| $buffer->seek(0); | ||
| if ($buffer->getSize() < $this->putSizeLimit) { | ||
| $streamSize = $psrStream->getSize(); | ||
| if ($psrStream->isSeekable() && (int)$streamSize !== 0) { |
There was a problem hiding this comment.
I find it a bit weird to cast here and not above.
It should work though because $buffer->getSize will overwrite it later.
juliusknorr
left a comment
juliusknorr
left a comment
There was a problem hiding this comment.
Blocking as this has been reported to still take quite some round trips on some setup
|
#38763 might be the reason why this wasn't working as expected on the mentioned setup |
|
Man, this would really help. Where did the stream behavior change? Is there a chance that I bakcport it already for our Next (V25) release or later - as MagentaCLOUD will hopefully move fast to V27 in fall? If I understand it right the behavior change comes from #34232, which is not merged to stable 24. I assume if I backport both, this could work, right? Thanks. |
|
#49352 is going into a similar direction and was merged now |
7 participants
On my local setup the stream was seekable and reported the proper size, so I think we can actually avoid the extra effort to copy the stream beginning to memory to determin which upload mechanism to use for cases where a size is reported. The fallback is still in place if the stream reports 0 or null as the size.
Introduced with https://github.com/nextcloud/server/pull/27877/files#diff-030e38a9e621019981acf91654c1d8d3a12ab3150dbd9fcc4c280479b8c853dbR144-R148
Test case needed adjustment since the implementation of a NonSeekableStream falsely reported to be seekable on its metadata.