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

Re: [PATCH v2] patch-ids: document intentional const-casting in patch_id_neq()

From
cat@malon.dev <cat@malon.dev>
Date
Mar 9, 2026, 06:39 UTC
Message-ID
<4d93dbf55e141460989f21edad24440d@purelymail.com>
In-Reply-To
<xmqqh5qp97bd.fsf@gitster.g>
Hi Junio,
Show 23 quoted lines
> Is that a "performance regression", I have to wonder?  We would
> regress relative to what by doing what?
> 
> Is the lazy evaluation avoiding unnecessary work?
> 
> If we are going to pass _all_ the objects in the hashmap to this
> comparator function eventually _anyway_, then the total cost of
> computing patch IDs to all of them in the hashmap would not change
> with or without lazy computation, but if we are currently getting
> away without having to compute for all, but only computing for the
> ones we pass to this function, then lazy evaluation is clearly a
> win.  I do not offhand know which of the above two is the case, but
> we need to know that before we can touch the NEEDSWORK comment, I
> think.
> 
> The lazy computation comes from b3dfeebb (rebase: avoid computing
> unnecessary patch IDs, 2016-07-29), even though the "const
> correctness?" comment is a bit newer than that.
> 
> So it seems that we indeed are avoiding unnecessary work without
> this patch.  We'd encounter "performance regression" only if we stop
> avoiding unnecessary work, so I am afraid that the phrasing used in
> the patch is somewhat confusing.

You're right. Avoiding unnecessary work is indeed a more fundamental reason than preventing performance regression.

Show 9 quoted lines
>     Even though eptr and entry_or_key are const, we want to lazily
>     compute their .patch_id members; see b3dfeebb (rebase: avoid
>     computing unnecessary patch IDs, 2016-07-29), so cast the
>     constness away with container_of().
> 
> or something, perhaps?
> 
>>  	struct diff_options *opt = (void *)cmpfn_data;
>>  	struct patch_id *a, *b;

I will incorporate your suggested phrasing and reference to the historical commit in v3.

Regards,
Yuchen
Previous: Junio C HamanoNext: Tian Yuchen
Message 6 of 7 in “patch-ids: achieve const correctness in patch_id_neq()”
  1. patch-ids: achieve const correctness in patch_id_neq()Tian Yuchen, Mar 8, 2026
  2. Junio C HamanoMar 8, 2026
  3. Tian YuchenMar 8, 2026
  4. patch-ids: document intentional const-casting in patch_id_neq()Tian Yuchen, Mar 8, 2026
  5. Junio C HamanoMar 9, 2026
  6. cat@malon.devMar 9, 2026
  7. patch-ids: document intentional const-casting in patch_id_neq()Tian Yuchen, Mar 9, 2026

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.