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

Re: [PATCH 4/7] delta_base_cache: use list.h for LRU

From
EWEric Wong <e@80x24.org>
Date
Aug 22, 2016, 23:18 UTC
Message-ID
<20160822231806.GA26148@starla>
In-Reply-To
<20160822215942.nm2jcrkyib7acr4u@sigill.intra.peff.net>
Jeff King <peff@peff.net> wrote:
Show 7 quoted lines
> We keep an LRU list of entries for when we need to drop
> something from an over-full cache. The list is implemented
> as a circular doubly-linked list, which is exactly what
> list.h provides. We can save a few lines by using the list.h
> macros and functions. More importantly, this makes the code
> easier to follow, as the reader sees explicit concepts like
> "list_add_tail()" instead of pointer manipulation.
Yay!
Show 6 quoted lines
> As a bonus, the list_entry() macro lets us place the lru
> pointers anywhere inside the delta_base_cache_entry struct
> (as opposed to just casting the pointer, which requires it
> at the front of the struct). This will be useful in later
> patches when we need to place other items at the front of
> the struct (e.g., our hashmap implementation requires this).

On a side note, I think we should s/list_entry/container_of/ and use container_of for hashmap.

Linux kernel defines list_entry to use container_of, but I'd rather avoid introducing the duality entirely.

Show 13 quoted lines
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> I think the result is much nicer, but I found list_entry() a little
> disappointing, because we lack typeof(). So you are stuck writing:
> 
>   struct delta_base_cache_entry *f =
>     list_entry(p, struct delta_base_cache_entry, lru);
> 
> I waffled on adding something like:
> 
>   LIST_ITEM(struct delta_bas_cache_entry, f, p, lru);
> 
> to declare "f" as above. But it's getting rather magical and un-C-like.

Right. I'd rather keep the list_entry/container_of usage identical to what developers familiar using userspace-rcu, ccan/list, or the Linux kernel expect.

>  sha1_file.c | 38 ++++++++++++++++----------------------
>  1 file changed, 16 insertions(+), 22 deletions(-)
Good to see the code reduction.
Previous: Jeff KingNext: Jeff King
Message 11 of 17 in “tweaking the delta base cache”
  1. 0/7 tweaking the delta base cacheJeff King, Aug 22, 2016
  2. 1/7 cache_or_unpack_entry: drop keep_cache parameterJeff King, Aug 22, 2016
  3. Junio C HamanoAug 23, 2016
  4. 2/7 clear_delta_base_cache_entry: use a more descriptive nameJeff King, Aug 22, 2016
  5. Junio C HamanoAug 23, 2016
  6. 3/7 release_delta_base_cache: reuse existing detach functionJeff King, Aug 22, 2016
  7. Junio C HamanoAug 23, 2016
  8. Jeff KingAug 24, 2016
  9. Junio C HamanoAug 24, 2016
  10. 4/7 delta_base_cache: use list.h for LRUJeff King, Aug 22, 2016
  11. Eric WongAug 22, 2016
  12. Jeff KingAug 23, 2016
  13. 5/7 delta_base_cache: drop special treatment of blobsJeff King, Aug 22, 2016
  14. Junio C HamanoAug 23, 2016
  15. 6/7 delta_base_cache: use hashmap.hJeff King, Aug 22, 2016
  16. Junio C HamanoAug 23, 2016
  17. 7/7 t/perf: add basic perf tests for delta base cacheJeff King, Aug 22, 2016

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.