On 19 Srpen 2014, 10:26, Jeff Davis wrote:
> On Sat, 2014-08-16 at 23:09 +0200, Tomas Vondra wrote:
>> But maybe the inheritance really is not necessary - maybe it would be
>> enough to track this per-context, and then just walk through the
>> contexts and collect this. Because my observation is that most of the
>> time is actually spent in walking through blocks and freelists.
>
> That makes a lot of sense to me.
>
> Another approach is to pass a flag to hash_create that tells it not to
> create a subcontext. Then we don't need to traverse anything; we already
> know which context we want to look at. Perhaps I was being too clever
> with the idea of tracking space for an entire hierarchy.
>
> Also, as I pointed out in my reply to Robert, adding too many fields to
> MemoryContextData may be the cause of the regression. Your idea requires
> only one field, which doesn't show the same regression in my tests.
Yeah, keeping the structure size below 64B seems like a good idea.
The use-case for this is tracking a chosen subtree of contexts - e.g.
aggcontext and below, so I'd expect the tracked subtrees to be relatively
shallow. Am I right?
My fear is that by removing the inheritance bit, we'll hurt cases with a
lot of child contexts. For example, array_agg currently creates a separate
context for each group - what happens if you have 100k groups and do
MemoryContextGetAllocated? I guess iterating over 100k groups is not free.
Wouldn't the solution with inheritance and propagating the accounting info
to the parent actually better? Or maybe allowing both, having two flags
when creating a context instead of one?
AllocSetCreateTracked( ...., bool track, bool propagate_immediately)
By squashing both flags into a single mask you wouldn't increase the size.
Also, do we really need to track allocated bytes - couldn't we track
kilobytes or something and use smaller data types to get below the 64B?
regards
Tomas