On 7 September 2015 at 11:46, Branko Čibej <brane_at_wandisco.com> wrote:
> On 07.09.2015 09:38, Ivan Zhakov wrote:
>> On 7 September 2015 at 10:30, Branko Čibej <brane_at_wandisco.com> wrote:
>>> On 07.09.2015 09:15, ivan_at_apache.org 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?
--
Ivan Zhakov
Received on 2015-09-07 10:52:49 CEST