On 8/28/2018 5:21 PM, Jeff King wrote:
On Sun, Aug 26, 2018 at 08:56:21PM +0000, brian m. carlson wrote:
Due to the simplicity of the current code and our inlining, the compiler
can usually figure this out for now. So I wouldn't expect this patch to
actually improve performance right away. But as that discussion shows,
we are likely to take a performance hit as we move to more runtime
determination of the_hash_algo parameters. Having these callers use the
more strict form will potentially help us recover that.
So in that sense we _could_ simply punt on this series until then (and
it's certainly post-v2.19 material). But I think it's worth doing now,
simply from a readability/annotation standpoint. IMHO the resulting code
is more clear (though I've long since taught myself to read !foocmp() as
equality).
I would quite like to see this series picked up for v2.20. If we want
to minimize performance regressions with the SHA-256 work, I think it's
important.
Thanks. One of the things I was worried about was causing unnecessary
conflicts with existing topics, including your work. But if everybody is
on board, I'd be happy to see this go in early in the next release cycle
(the longer we wait, the more annoying conflicts Junio has to resolve).
I'm happy to take this change whenever. In my opinion, right after
v2.19.0 is cut would be a great time to merge it into master.
This v2 is good.
Reviewed-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>