Re: Backend memory dump analysis

Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> writes:
> How about this one as well:

> portal->portalContext = AllocSetContextCreate(TopPortalContext,
> "PortalContext",
> ALLOCSET_SMALL_SIZES);
> + MemoryContextCopySetIdentifier(portal->portalContext, name);

Seems reasonable, although I think if you were to delay setting the
name till the end of that function, you could point to portal->name
and avoid the extra pstrdup. Maybe that's useless microoptimization.

> The term CopySetIdentifier has confused me a bit. (What's a "set
> identifier"?) Maybe use CopyAndSetIdentifier? (We similarly have
> MemoryContextResetAndDeleteChildren.)

No objection, do you want to make the change?

> I'm also not clear why this doesn't undo the previous optimization that
> preferred making the identifier a compile time-constant. Aren't we now
> just going back to doing a pstrdup() every time?

Huh? It's not undoing that, it's doubling down on it; the "name" now
*has* to be a compile-time constant. Only for contexts that seem worthy
of carrying extra ID information, which is a small minority, do we bother
setting the ident field. Even for those, in the majority of cases we can
avoid an extra strcpy because the identity info is being carried somewhere
inside the context already.

regards, tom lane