On 7 September 2015 at 12:25, Branko Čibej <brane_at_wandisco.com> wrote:
> On 07.09.2015 10:52, Ivan Zhakov wrote:
>> 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?
>
>
> 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
Received on 2015-09-07 11:29:33 CEST