Re: Backend memory dump analysis - Mailing list pgsql-hackers

Andres Freund <andres@anarazel.de> writes:
> On 2018-03-23 18:05:38 +0000, Vladimir Sitnikov wrote:
>> For instance, I assume statament cache is stored in some sort of a hash
>> table, so there should be a way to enumerate it in a programmatic way. Of
>> course it would take time, however I do not think it creates cpu/memory
>> overheads. The overhead is to maintain "walker" code.

> Sure, you could, entirely independent of the memory stats dump, do
> that. But what information would you actually gain from it? Which row
> something in the catcache belongs to isn't *that* interesting.

It'd certainly be easy to define this in a way that makes it require
a bunch of support code, which we'd be unlikely to want to write and
maintain.  However, I've often wished that the contexts in a memory
dump were less anonymous.  If you didn't just see a pile of "PL/pgSQL
function context" entries, but could (say) see the name of each function,
that would be a big step forward.  Similarly, if we could see the source
text for each CachedPlanSource in a dump, that'd be useful.  I mention
these things because we do actually store them already, in many cases
--- but the memory stats code doesn't know about them.

Now, commit 9fa6f00b1 already introduced a noticeable penalty for
contexts with nonconstant names, so trying to stick extra info like
this into the context name is not appetizing.  But what if we allowed
the context name to have two parts, a fixed part and a variable part?
We could actually require that the fixed part be a compile-time-constant
string, simplifying matters on that end.  The variable part would best
be assigned later than initial context creation, because you'd need a
chance to copy the string into the context before pointing to it.
So maybe, for contexts where this is worth doing, it'd look something
like this for plpgsql:

    func_cxt = AllocSetContextCreate(TopMemoryContext,
                                     "PL/pgSQL function context",
                                     ALLOCSET_DEFAULT_SIZES);
    plpgsql_compile_tmp_cxt = MemoryContextSwitchTo(func_cxt);

    function->fn_signature = format_procedure(fcinfo->flinfo->fn_oid);
+   MemoryContextSetIdentifier(func_cxt, function->fn_signature);
    function->fn_oid = fcinfo->flinfo->fn_oid;
    function->fn_xmin = HeapTupleHeaderGetRawXmin(procTup->t_data);

This would cost an extra char * field in struct MemoryContextData,
which is slightly annoying but it doesn't exactly seem like a killer.
Then the memory stats dump code would just need to know to print this
field if it isn't NULL.

If we wanted to do this I'd suggest sneaking it into v11, so that
if people have to adapt their code because of 9fa6f00b1 breaking
usages with nonconstant context names, they have a solution to turn to
immediately rather than having to change things again in v12.

Thoughts?

            regards, tom lane