On 2013-12-13 13:39:20 -0300, Alvaro Herrera wrote:
> Here's cache code with LRU superpowers (ahem.)
Heh.
> I settled on 256 as number of entries because it's in the same ballpark
> as MaxHeapTuplesPerPage which seems a reasonable guideline to follow.
Sounds ok.
> I considered the idea of avoiding palloc/pfree for cache entries
> entirely, instead storing them in a static array which is referenced
> from the dlist; unfortunately that doesn't work because each cache entry
> is variable size, depending on number of members. We could try to work
> around that and allocate a large shared array for members, but that
> starts to smell of over-engineering, so I punted.
Good plan imo.
> *** 1326,1331 **** mXactCacheGetBySet(int nmembers, MultiXactMember *members)
> --- 1331,1337 ----
> if (memcmp(members, entry->members, nmembers * sizeof(MultiXactMember)) == 0)
> {
> debug_elog3(DEBUG2, "CacheGet: found %u", entry->multi);
> + dlist_move_head(&MXactCache, iter.cur);
> return entry->multi;
> }
> }
That's only possible because we immediately abort the loop, otherwise
we'd corrupt the iterator. Maybe that deserves a comment.
> +
> + dlist_move_head(&MXactCache, iter.cur);
> +
Heh. I forgot that we already had that bit; I was wondering whether you
had to forgot to include it in the patch ;)
> static char *
> --- 1420,1435 ----
> /* mXactCacheGetBySet assumes the entries are sorted, so sort them */
> qsort(entry->members, nmembers, sizeof(MultiXactMember), mxactMemberComparator);
>
> ! dlist_push_head(&MXactCache, &entry->node);
> ! if (MXactCacheMembers++ >= MAX_CACHE_ENTRIES)
> ! {
> ! dlist_node *node;
> !
> ! node = dlist_tail_node(&MXactCache);
> ! dlist_delete(dlist_tail_node(&MXactCache));
> ! MXactCacheMembers--;
> ! pfree(dlist_container(mXactCacheEnt, node, node));
> ! }
> }
Duplicate dlist_tail_node(). Maybe add a debug_elog3(.. "CacheGet:
pruning %u from cache")?
I wondered before if we shouldn't introduce a layer above dlists, that
support keeping track of the number of elements, and maybe also have
support for LRU behaviour. Not as a part this patch, just generally.
Greetings,
Andres Freund
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services