• David Rowley's avatar
    Move memory accounting Asserts for Result Cache code · 92c4c269
    David Rowley authored
    In 9eacee2e, I included some code to verify the cache's memory tracking
    is correct by counting up the number of entries and the memory they use
    each time we evict something from the cache.  Those values are then
    compared to the expected values using Assert.  The problem is that this
    requires looping over the entire cache hash table each time we evict an
    entry from the cache.  That can be pretty expensive, as noted by Pavel
    Stehule.
    
    Here we move this memory accounting checking code so that we only verify
    it on cassert builds once when shutting down the Result Cache node.
    
    Aside from the performance increase, this has two distinct advantages:
    
    1) We do the memory checks at the last possible moment before destroying
       the cache.  This means we'll now catch accounting problems that might
       sneak in after a cache eviction.
    
    2) We now do the memory Assert checks when there were no cache evictions.
       This increases the coverage.
    
    One small disadvantage is that we'll now miss any memory tracking issues
    that somehow managed to resolve themselves by the end of execution.
    However, it seems to me that such a memory tracking problem would be quite
    unlikely, and likely somewhat less harmful if one were to exist.
    
    In passing, adjust the loop over the hash table to use the standard
    simplehash.h method of iteration.
    
    Reported-by: Pavel Stehule
    Discussion: https://postgr.es/m/CAFj8pRAzgoSkdEiqrKbT=7yG9FA5fjUAP3jmJywuDqYq6Ki5ug@mail.gmail.com
    92c4c269
nodeResultCache.c 33.7 KB