git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 0/9] introducing oideq()

From
Derrick Stolee <stolee@gmail.com>
Date
Aug 27, 2018, 12:41 UTC
Message-ID
<dd26d177-7e37-83ac-7275-03db962a282c@gmail.com>
In-Reply-To
<20180826205620.GC873448@genre.crustytoothpaste.net>
On 8/26/2018 4:56 PM, brian m. carlson wrote:
Show 25 quoted lines
> On Sat, Aug 25, 2018 at 04:00:31AM -0400, Jeff King wrote:
>> This is a follow-up to the discussion in:
>>
>>    https://public-inbox.org/git/20180822030344.GA14684@sigill.intra.peff.net/
>>
>> The general idea is that the majority of callers don't care about actual
>> plus/minus ordering from oidcmp() and hashcmp(); they just want to know
>> if two oids are equal. The stricter equality check can be optimized
>> better by the compiler.
>>
>> 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.
Seconded.
> Applying the following patch on top of this series causes gcc to inline
> both branches, which is pretty much the best we can expect.  I haven't
> benchmarked it, though, so I can't say what the actual performance
> consequence is.

We should definitely measure the cost here, but when we have the option to move to newhash, that cost should be acceptable.

Thanks,
-Stolee
Previous: brian m. carlsonNext: Jeff King
Message 18 of 30 in “introducing oideq()”
  1. 0/9 introducing oideq()Jeff King, Aug 25, 2018
  2. 1/9 coccinelle: use <...> for function exclusionJeff King, Aug 25, 2018
  3. 2/9 introduce hasheq() and oideq()Jeff King, Aug 25, 2018
  4. Andrei RybakAug 25, 2018
  5. 3/9 convert "oidcmp() == 0" to oideq()Jeff King, Aug 25, 2018
  6. Jeff KingAug 25, 2018
  7. Derrick StoleeAug 27, 2018
  8. Derrick StoleeAug 27, 2018
  9. 4/9 convert "hashcmp() == 0" to hasheq()Jeff King, Aug 25, 2018
  10. 5/9 convert "oidcmp() != 0" to "!oideq()"Jeff King, Aug 25, 2018
  11. 6/9 convert "hashcmp() != 0" to "!hasheq()"Jeff King, Aug 25, 2018
  12. 7/9 convert hashmap comparison functions to oideq()Jeff King, Aug 25, 2018
  13. 8/9 read-cache: use oideq() in ce_compare functionsJeff King, Aug 25, 2018
  14. 9/9 show_dirstat: simplify same-content checkJeff King, Aug 25, 2018
  15. Eric SunshineAug 25, 2018
  16. Jeff KingAug 25, 2018
  17. brian m. carlsonAug 26, 2018
  18. Derrick StoleeAug 27, 2018
  19. Jeff KingAug 28, 2018
  20. 1/9 coccinelle: use <...> for function exclusionJeff King, Aug 28, 2018
  21. 2/9 introduce hasheq() and oideq()Jeff King, Aug 28, 2018
  22. 3/9 convert "oidcmp() == 0" to oideq()Jeff King, Aug 28, 2018
  23. 4/9 convert "hashcmp() == 0" to hasheq()Jeff King, Aug 28, 2018
  24. 5/9 convert "oidcmp() != 0" to "!oideq()"Jeff King, Aug 28, 2018
  25. 6/9 convert "hashcmp() != 0" to "!hasheq()"Jeff King, Aug 28, 2018
  26. 7/9 convert hashmap comparison functions to oideq()Jeff King, Aug 28, 2018
  27. 8/9 read-cache: use oideq() in ce_compare functionsJeff King, Aug 28, 2018
  28. 9/9 show_dirstat: simplify same-content checkJeff King, Aug 28, 2018
  29. Derrick StoleeAug 28, 2018
  30. brian m. carlsonAug 29, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.