• Tom Lane's avatar
    Allow memory contexts to have both fixed and variable ident strings. · 442accc3
    Tom Lane authored
    Originally, we treated memory context names as potentially variable in
    all cases, and therefore always copied them into the context header.
    Commit 9fa6f00b rethought this a little bit and invented a distinction
    between fixed and variable names, skipping the copy step for the former.
    But we can make things both simpler and more useful by instead allowing
    there to be two parts to a context's identification, a fixed "name" and
    an optional, variable "ident".  The name supplied in the context create
    call is now required to be a compile-time-constant string in all cases,
    as it is never copied but just pointed to.  The "ident" string, if
    wanted, is supplied later.  This is needed because typically we want
    the ident to be stored inside the context so that it's cleaned up
    automatically on context deletion; that means it has to be copied into
    the context before we can set the pointer.
    
    The cost of this approach is basically just an additional pointer field
    in struct MemoryContextData, which isn't much overhead, and is bought
    back entirely in the AllocSet case by not needing a headerSize field
    anymore, since we no longer have to cope with variable header length.
    In addition, we can simplify the internal interfaces for memory context
    creation still further, saving a few cycles there.  And it's no longer
    true that a custom identifier disqualifies a context from participating
    in aset.c's freelist scheme, so possibly there's some win on that end.
    
    All the places that were using non-compile-time-constant context names
    are adjusted to put the variable info into the "ident" instead.  This
    allows more effective identification of those contexts in many cases;
    for example, subsidary contexts of relcache entries are now identified
    by both type (e.g. "index info") and relname, where before you got only
    one or the other.  Contexts associated with PL function cache entries
    are now identified more fully and uniformly, too.
    
    I also arranged for plancache contexts to use the query source string
    as their identifier.  This is basically free for CachedPlanSources, as
    they contained a copy of that string already.  We pay an extra pstrdup
    to do it for CachedPlans.  That could perhaps be avoided, but it would
    make things more fragile (since the CachedPlanSource is sometimes
    destroyed first).  I suspect future improvements in error reporting will
    require CachedPlans to have a copy of that string anyway, so it's not
    clear that it's worth moving mountains to avoid it now.
    
    This also changes the APIs for context statistics routines so that the
    context-specific routines no longer assume that output goes straight
    to stderr, nor do they know all details of the output format.  This
    is useful immediately to reduce code duplication, and it also allows
    for external code to do something with stats output that's different
    from printing to stderr.
    
    The reason for pushing this now rather than waiting for v12 is that
    it rethinks some of the API changes made by commit 9fa6f00b.  Seems
    better for extension authors to endure just one round of API changes
    not two.
    
    Discussion: https://postgr.es/m/CAB=Je-FdtmFZ9y9REHD7VsSrnCkiBhsA4mdsLKSPauwXtQBeNA@mail.gmail.com
    442accc3
extended_stats.c 13.4 KB