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.
-- Brane
Received on 2015-09-07 11:25:50 CEST