On 07.09.2015 10:52, Ivan Zhakov wrote: > On 7 September 2015 at 11:46, Branko Čibej <[email protected]> wrote: >> On 07.09.2015 09:38, Ivan Zhakov wrote: >>> On 7 September 2015 at 10:30, Branko Čibej <[email protected]> wrote: >>>> On 07.09.2015 09:15, [email protected] wrote: >>>>> Author: ivan >>>>> Date: Mon Sep 7 07:15:25 2015 >>>>> New Revision: 1701564 >>>>> >>>>> URL: http://svn.apache.org/r1701564 >>>>> Log: >>>>> Reduce difference between Windows and non-Windows implementation of >>>>> install_stream. >>>>> >>>>> * subversion/libsvn_subr/stream.c >>>>> (install_close): Remove #ifdef WIN32 >>>>> (svn_stream__create_for_install): Always setup flush on close stream >>>>> handler. >>>>> (svn_stream__install_stream): Close temporary file before rename on all >>>>> platforms. >>>>> (svn_stream__install_get_info): Obtain file info from file handle on all >>>>> platforms. >>>>> (svn_stream__install_delete): Close temporary file before delete on all >>>>> platforms. >>>> Why do you think this is a good idea? I know you can't move/delete an >>>> open file on Windows without jumping through hoops that APR doesn't >>>> provide, but you certainly can do that on Unix and if memory serves, >>>> that actually helps performance in some cases where remote file systems >>>> are involved. >>>> >>> Before this patch temporary file was closed anyway before >>> rename/delete in svn_stream_close() handler for temporary file on >>> non-Windows platform. Windows specific code postponed this to >>> rename/delete stage. The r1701564 makes this code the same on Windows >>> and non-Windows. >> Yes, I understand that; what I don't understand is why you think this >> change was necessary. AFAIK it just causes a slight performance hit on >> Unix platforms with remote file systems. >> > What change do you mean? The actual operations performed on unix > didn't changed in this commit (except obtaining finfo from handle > instead of by path), only the code changed. Before this change > temporary files was closed in svn_stream_close() which is called > before svn_stream__install_stream/svn_stream__install_delete. After > this change it's performed in > svn_stream__install_stream/svn_stream__install_delete itself before > rename/delete. > > What performance hit do you mean?
Looks like I'm stupid today and managed to read the diff the wrong way around. Sorry ... just ignore me until I get my head screwed on correctly again. -- Brane

