[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1169837 - /subversion/trunk/subversion/libsvn_delta/compat.c

From: Hyrum K Wright <hyrum.wright_at_wandisco.com>
Date: Mon, 12 Sep 2011 16:11:04 -0500

On Mon, Sep 12, 2011 at 2:06 PM, Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:
> Look.  You're just wrong.  SVN_ERR() always implies svn_error_trace()
> regardless of SVN_ERR__TRACING, and there is no additional branch here
> in an optimized build (and last time you were introducing those changes
> someone demonstrated this via disassembly excerpts).

As you point out, the removal of the additional branch depends upon
the optimizer. I hardly think that means I'm "just wrong." Partially
wrong, maybe, but it certainly isn't as final as you make it sound. :)

In any case, I was making a similar change elsewhere, and decided to
remove some extra cruft. You may not agree with that change, and
that's fine. But right now, this is all very new code, and
(unfortunately) I'm the only person actively hacking it, so I'm kinda
inclined to do what makes my life easier, so long as it doesn't hurt
anything else.

> So, if you want to reduce on branches, then merge ^/subversion/branches/performance.
> But as I see it such changes make the code less readable for no gain.

And I see it as making the code more readable, which *is* a gain. But
that's just a personal preference upon which we can agree to disagree.


uberSVN: Apache Subversion Made Easy
Received on 2011-09-12 23:11:38 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.