Re: Optimize WindowAgg's use of tuplestores - Mailing list pgsql-hackers

From Tatsuo Ishii
Subject Re: Optimize WindowAgg's use of tuplestores
Date
Msg-id 20240712.122020.999809343326222897.ishii@postgresql.org
Whole thread Raw
In response to Re: Optimize WindowAgg's use of tuplestores  (David Rowley <dgrowleyml@gmail.com>)
Responses Re: Optimize WindowAgg's use of tuplestores
List pgsql-hackers
Hi David,

Thank you for the patch.

> I think this might just be noise as a result of rearranging code. In
> terms of C code, I don't see any reason for it to be slower.  If you
> look at GenerationDelete() (as what is getting called from
> MemoryContextDelete()), it just calls GenerationReset(). So resetting
> is going to always be less work than deleting the context, especially
> given we don't need to create the context again when we reset it.
> 
> I wrote the attached script to see if I can also see the slowdown and
> I do see the patched code come out slightly slower (within noise
> levels) in lower partition counts.
> 
> To get my compiler to produce code in a more optimal order for the
> common case, I added unlikely() to the "if (winstate->all_first)"
> condition.  This is only evaluated on the first time after a rescan,
> so putting that code at the end of the function makes more sense.  The
> attached v2 patch has it this way.  You can see the numbers look
> slightly better in the attached graph.
> 
> Thanks for having a look at this.
> 
> David

@@ -2684,6 +2726,14 @@ ExecEndWindowAgg(WindowAggState *node)
     PlanState  *outerPlan;
     int            i;
 
+    if (node->buffer != NULL)
+    {
+        tuplestore_end(node->buffer);
+
+        /* nullify so that release_partition skips the tuplestore_clear() */
+        node->buffer = NULL;
+    }
+

Is it possible that node->buffer == NULL in ExecEndWindowAgg()?  If
not, probably making it an Assert() or just removing the if() should
be fine.

Best reagards,
--
Tatsuo Ishii
SRA OSS LLC
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp



pgsql-hackers by date:

Previous
From: Andrei Lepikhov
Date:
Subject: Re: Check lateral references within PHVs for memoize cache keys
Next
From: Thomas Munro
Date:
Subject: Re: Clang function pointer type warnings in v14, v15