> -----Original Message-----
> From: Ben Reser [mailto:ben_at_reser.org]
> Sent: vrijdag 26 juli 2013 04:53
> To: Bert Huijben
> Cc: Subversion Development; rschupp_at_apache.org;
> commits_at_subversion.apache.org
> Subject: Re: svn commit: r1507044 -
>
> On Thu, Jul 25, 2013 at 5:26 PM, Ben Reser <ben_at_reser.org> wrote:
> > There are far fewer cases where svn_hash_gets() results in extra work.
> > At first I thought it might not be worth fixing it since you have to
> > turn it into a function. But then I ran into the cases in libsvn_wc
> > and libsvn_delta and I think we should fix it as well.
>
> So I'm not sure what to do about the svn_hash_gets() case. If I
> convert it to a function it's no longer optimal for literal strings
> (unless the compiler is inlining it, which it probably won't be
> because it's an external function). In which case I might as well
> just put it back as:
> #define svn_hash_gets apr_hash_get(ht, key, APR_HASH_KEY_STRING, val)
>
> If we don't change it then we need to change all the callers to be
> careful about not putting expressions that would be an issue when
> evaluated twice, which is almost certainly going to be done wrong at
> some point in the future.
>
> If we do change it then we'll probably end up using apr_hash_get()
> with literal strings because we end up with more optimal code in those
> cases. The upside here is that even if we end up using svn_hash_get()
> where it would be less optimal to do so, it probably won't be as bad
> as using svn_hash_gets() as it exists now with a function call as the
> key expression.
>
> Anyone else got some clever idea on how to fix it that I haven't thought
of?
I think you covered any case I can think of.
I think going back to
#define svn_hash_gets apr_hash_get(ht, key, APR_HASH_KEY_STRING, val)
is probably the safest thing to do.
In the original discussions around improving svn_hash_gets with other
variants for improved performance, we already determined that most of this
would only be relevant for certain performance critical cases.
This all goes much further than the original idea of simply making it much
easier to write than the extreme common apr_hash_get(ht, key,
APR_HASH_KEY_STRING, val).
Bert
Received on 2013-07-26 11:35:59 CEST