Re: svn commit: r1698359 - in /subversion/trunk/subversion: include/svn_io.h libsvn_subr/stream.c svnadmin/svnadmin.c svnfsfs/load-index-cmd.c tests/libsvn_subr/stream-test.c

Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com> writes:

>> Given the above, I am -1 on this optimization for svnadmin load-revprops
>> that was implemented in r1698359 and r1700305. Please revert these
>> changes.
>
> Thinking about alternative solutions I found that simply having a buffering
> wrapper without mark support would still eliminate the OS overhead and parts
> of the internal overhead. That would address all the points you have made
> above while still providing a decent improvement.
>
> IOW, revert r1700305 and rework / reduce / simplify the code changed
> by r1698359.

I stated my veto and provided the justification that covers both r1700305
and r1698359. So, could you please revert both of them? Reworking and
adding changes on top of it is going to increase the mess and will be harder
to review.

> I wanted to add -F for a while now because it makes debugging much easier
> (when using IDEs). Never got around it, though.
>
> So, feel free to add -F support to load and load-revprops.

Frankly, I don't have the numbers that would prove the noticeable performance
benefit from replacing byte-by-byte parsing with chunk-based parsing. So, this
is not on my current priority list.

Regards,
Evgeny Kotkov
Received on 2015-09-02 14:44:19 CEST