Re: Optimize WindowAgg's use of tuplestores
David Rowley <dgrowleyml@gmail.com>
From: David Rowley <dgrowleyml@gmail.com>
To: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Cc: David Rowley <dgrowley@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2024-08-19T10:01:22Z
Lists: pgsql-hackers
Attachments
- v3-0001-Speedup-WindowAgg-code-by-moving-uncommon-code-ou.patch (application/octet-stream) patch v3-0001
- v3-0002-Optimize-WindowAgg-s-use-of-tuplestores.patch (application/octet-stream) patch v3-0002
- appleM2_windowagg.png (image/png)
- amd7945hx_windowagg.png (image/png)
- Ashutosh_windowagg.png (image/png)
On Mon, 15 Jul 2024 at 18:02, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote: > There is significant improvement with a large number of partitions as > seen previously. But for a smaller number of partitions the > performance worsens, which needs some investigation. We get performance variations all the time from unrelated changes due to alignment changes in the code layout. There's a write-up in [1] that you might find interesting. In particular the following paragraph: "Unfortunately, modern architectural features make this approach unsound. Statistically sound evaluation requires multiple samples to test whether one can or cannot (with high confidence) reject the null hypothesis that results are the same before and after. However, caches and branch predictors make performance dependent on machine-specific parameters and the exact layout of code, stack frames, and heap objects. A single binary constitutes just one sample from the space of program layouts, regardless of the number of runs. Since compiler optimizations and code changes also alter layout, it is currently impossible to distinguish the impact of an optimization from that of its layout effects." Since the code changes here add no additional work, they only remove work. I think any regressions you see must be related to code alignment. To try and move this forward again, I adjusted the patch to use a static function with pg_noinline rather than unlikely. I don't think this will make much difference code generation wise, but I did think it was an improvement in code cleanliness. Patches attached. I did a round of benchmarking on an AMD Zen4 7945hx and on an Apple M2. I also graphed the results you sent so they're easier to compare with mine. 0001 is effectively the unlikely() patch for calculating the frame offsets. 0002 is the tuplestore_reset() patch The AMD laptop results were a bit noisy. M2 results were much more stable. David [1] https://emeryberger.com/research/stabilizer/
Commits
-
Optimize WindowAgg's use of tuplestores
- 908a968612f9 18.0 landed
-
Speedup WindowAgg code by moving uncommon code out-of-line
- 19b861f88016 18.0 landed
-
Improve memory management and performance of tuplestore.c
- 590b045c37aa 18.0 cited