Ivan Zhakov wrote on Wed, Sep 02, 2015 at 18:39:38 +0300:
> On 2 September 2015 at 17:01, Johan Corveleyn <jcorvel_at_gmail.com> wrote:
> > On Wed, Sep 2, 2015 at 2:43 PM, Evgeny Kotkov
> > <evgeny.kotkov_at_visualsvn.com> wrote:
> >> 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.
> >
> > ISTR that in this community we try to treat a veto as a signal that
> > further discussion is needed, or that more work is needed to resolve
> > the question / issue. You may ask / suggest a revert if you think
> > that's best, but it's mainly up for discussion how to best resolve
> > things (and other avenues, such as further commits, may also resolve
> > the issue).
> >
> You're right, but the problem is that the discussion doesn't happen.
> Instead of this new code is committed (usually broken again) [1].
>
> Let summarize what happened from my point of view:
> 1. An optimization was implemented in r1698359
> 2. Evgeny raised his concerns about possible unbounded memory usage
> 3. Some fix was applied in r1700305 (no discussion happened)
> 4. Evgeny vetoed both of them.
> 5. The original optimization was reworked (no discussion happened)
Vetoes do not require the vetoed code to be reverted.
Vetoes do not require the vetoed codepath to be frozen.
Vetoes require the community to resolve the technical concernes
underlying them before the next release.
Rewriting the code is a form of discussion. That discussion is
continuing on the r1700799 thread which you linked. Let's wait for
Stefan2 to reply on that thread to Evgeny's technical concerns with the
rewrite.
> 6. Another problem was found in the new code [1]
> 7. ???
What's the question? Do what you always do whenever a problem is found
in the code: review the code, reach consensus on a fix, implement it;
iteratively as needed. That's exactly what the r1700799 thread is.
Cheers,
Daniel
P.S. I haven't reviewed the technical content of the commits in
question.
Received on 2015-09-02 18:50:08 CEST