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

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.

-Hyrum

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/