@jamesgpearce James, these are very nice optimizations, but I can't really read the code generation code. The benchmark results visualization is very cool. I'm curious how did you create it? The changes looks good, but I wonder whether there is sufficient testing. Have you done any additional testing beyond unit tests? If so, would you share what these are?

CC: @tdcmeehan @kaikalur @yuanzhanhku

The benchmark results visualization is very cool. I'm curious how did you create it?

You can tell JMH to output results as JSON format, and those files can be parsed and displayed using jmh.morethan.io either by uploading the local file or by adding a ?sources=<url_one>,<url_two> query parameter (I uploaded mine as a github gist and to those URLs here).

The changes looks good, but I wonder whether there is sufficient testing. Have you done any additional testing beyond unit tests? If so, would you share what these are?

I added a test so that any distinct aggregations tests using AggregationTestUtils#distinctAggregation (called via AggregationTestUtils#assertAggregation) will test both RLE and non-RLE mode of distinct masks, but since I saw you were the last person to add changes to AccumulatorCompiler in #14123 that you might have a better sense about whether there were scenarios to test beyond the unit tests.

Thank you, James. We are consistently finding that deep changes in the engine like these are not getting sufficiently covered by unit tests as bugs come out when we run the verifier during release process or after deployment in production settings. Any chance you could run this change on some of the production workload you have access to? This would give us a bit more confidence that this change won't break things. That said, I feel the quality of this PR is high therefore I'm going to accept. I'll wait for you to decide whether you can do additional testing before merging.

This has been running in our test environment for a while now with no edge cases discovered and has also been merged into Trino at this point without incident reported there. Since I see other ongoing work in the area of aggregations and codegen, I'm going to merge once the rebased changes are green since it seems like letting this PR sit open longer is more likely to let incompatible changes creep in.