On 7 September 2015 at 12:25, Branko Čibej <[email protected]> wrote: > 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. > No problem. Diff sometimes is very confusing :)
-- Ivan Zhakov
