database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-11-25 12:03:53 +03:00
Code Activity

Allow memory contexts to have both fixed and variable ident strings.

Browse Source 
Originally, we treated memory context names as potentially variable in
all cases, and therefore always copied them into the context header.
Commit 9fa6f00b1 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 9fa6f00b1.  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
This commit is contained in:
Tom Lane
2018-03-27 16:46:47 -04:00
parent c203d6cf81
commit 442accc3fe
20 changed files with 261 additions and 208 deletions
Download Patch File Download Diff File Expand all files Collapse all files

138
src/backend/utils/mmgr/mcxt.c
View File

@@ -21,6 +21,7 @@
#include "postgres.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
#include "utils/memdebug.h"
#include "utils/memutils.h"
@@ -55,6 +56,8 @@ static void MemoryContextCallResetCallbacks(MemoryContext context);
static void MemoryContextStatsInternal(MemoryContext context, int level,
bool print, int max_children,
MemoryContextCounters *totals);
static void MemoryContextStatsPrint(MemoryContext context, void *passthru,
const char *stats_string);
/*
* You should not do memory allocations within a critical section, because
@@ -118,7 +121,6 @@ MemoryContextInit(void)
*/
ErrorContext = AllocSetContextCreateExtended(TopMemoryContext,
"ErrorContext",
0,
8 * 1024,
8 * 1024,
8 * 1024);
@@ -158,6 +160,17 @@ MemoryContextResetOnly(MemoryContext context)
if (!context->isReset)
{
MemoryContextCallResetCallbacks(context);
/*
* If context->ident points into the context's memory, it will become
* a dangling pointer. We could prevent that by setting it to NULL
* here, but that would break valid coding patterns that keep the
* ident elsewhere, e.g. in a parent context. Another idea is to use
* MemoryContextContains(), but we don't require ident strings to be
* in separately-palloc'd chunks, so that risks false positives. So
* for now we assume the programmer got it right.
*/
context->methods->reset(context);
context->isReset = true;
VALGRIND_DESTROY_MEMPOOL(context);
@@ -222,6 +235,13 @@ MemoryContextDelete(MemoryContext context)
*/
MemoryContextSetParent(context, NULL);
/*
* Also reset the context's ident pointer, in case it points into the
* context. This would only matter if someone tries to get stats on the
* (already unlinked) context, which is unlikely, but let's be safe.
*/
context->ident = NULL;
context->methods->delete_context(context);
VALGRIND_DESTROY_MEMPOOL(context);
@@ -295,6 +315,23 @@ MemoryContextCallResetCallbacks(MemoryContext context)
}
}
/*
* MemoryContextSetIdentifier
* Set the identifier string for a memory context.
*
* An identifier can be provided to help distinguish among different contexts
* of the same kind in memory context stats dumps. The identifier string
* must live at least as long as the context it is for; typically it is
* allocated inside that context, so that it automatically goes away on
* context deletion. Pass id = NULL to forget any old identifier.
*/
void
MemoryContextSetIdentifier(MemoryContext context, const char *id)
{
AssertArg(MemoryContextIsValid(context));
context->ident = id;
}
/*
* MemoryContextSetParent
* Change a context to belong to a new parent (or no parent).
@@ -480,7 +517,10 @@ MemoryContextStatsInternal(MemoryContext context, int level,
AssertArg(MemoryContextIsValid(context));
/* Examine the context itself */
context->methods->stats(context, level, print, totals);
context->methods->stats(context,
print ? MemoryContextStatsPrint : NULL,
(void *) &level,
totals);
/*
* Examine children. If there are more than max_children of them, we do
@@ -531,6 +571,67 @@ MemoryContextStatsInternal(MemoryContext context, int level,
}
}
/*
* MemoryContextStatsPrint
* Print callback used by MemoryContextStatsInternal
*
* For now, the passthru pointer just points to "int level"; later we might
* make that more complicated.
*/
static void
MemoryContextStatsPrint(MemoryContext context, void *passthru,
const char *stats_string)
{
int level = *(int *) passthru;
const char *name = context->name;
const char *ident = context->ident;
int i;
/*
* It seems preferable to label dynahash contexts with just the hash table
* name. Those are already unique enough, so the "dynahash" part isn't
* very helpful, and this way is more consistent with pre-v11 practice.
*/
if (ident && strcmp(name, "dynahash") == 0)
{
name = ident;
ident = NULL;
}
for (i = 0; i < level; i++)
fprintf(stderr, " ");
fprintf(stderr, "%s: %s", name, stats_string);
if (ident)
{
/*
* Some contexts may have very long identifiers (e.g., SQL queries).
* Arbitrarily truncate at 100 bytes, but be careful not to break
* multibyte characters. Also, replace ASCII control characters, such
* as newlines, with spaces.
*/
int idlen = strlen(ident);
bool truncated = false;
if (idlen > 100)
{
idlen = pg_mbcliplen(ident, idlen, 100);
truncated = true;
}
fprintf(stderr, ": ");
while (idlen-- > 0)
{
unsigned char c = *ident++;
if (c < ' ')
c = ' ';
fputc(c, stderr);
}
if (truncated)
fprintf(stderr, "...");
}
fputc('\n', stderr);
}
/*
* MemoryContextCheck
* Check all chunks in the named context.
@@ -612,34 +713,23 @@ MemoryContextContains(MemoryContext context, void *pointer)
*
* node: the as-yet-uninitialized common part of the context header node.
* tag: NodeTag code identifying the memory context type.
* size: total size of context header including context-type-specific fields,
* as well as space for the context name if MEMCONTEXT_COPY_NAME is set.
* nameoffset: where within the "size" space to insert the context name.
* methods: context-type-specific methods (usually statically allocated).
* parent: parent context, or NULL if this will be a top-level context.
* name: name of context (for debugging only, need not be unique).
* flags: bitmask of MEMCONTEXT_XXX option flags.
* name: name of context (must be statically allocated).
*
* Context routines generally assume that MemoryContextCreate can't fail,
* so this can contain Assert but not elog/ereport.
*/
void
MemoryContextCreate(MemoryContext node,
NodeTag tag, Size size, Size nameoffset,
NodeTag tag,
const MemoryContextMethods *methods,
MemoryContext parent,
const char *name,
int flags)
const char *name)
{
/* Creating new memory contexts is not allowed in a critical section */
Assert(CritSectionCount == 0);
/* Check size is sane */
Assert(nameoffset >= sizeof(MemoryContextData));
Assert((flags & MEMCONTEXT_COPY_NAME) ?
size >= nameoffset + strlen(name) + 1 :
size >= nameoffset);
/* Initialize all standard fields of memory context header */
node->type = tag;
node->isReset = true;
@@ -647,22 +737,10 @@ MemoryContextCreate(MemoryContext node,
node->parent = parent;
node->firstchild = NULL;
node->prevchild = NULL;
node->name = name;
node->ident = NULL;
node->reset_cbs = NULL;
if (flags & MEMCONTEXT_COPY_NAME)
{
/* Insert context name into space reserved for it */
char *namecopy = ((char *) node) + nameoffset;
node->name = namecopy;
strcpy(namecopy, name);
}
else
{
/* Assume the passed-in name is statically allocated */
node->name = name;
}
/* OK to link node into context tree */
if (parent)
{