On 11/1/24 06:17, David Rowley wrote:
> On Thu, 31 Oct 2024 at 15:30, Andrei Lepikhov <lepihov(at)gmail(dot)com> wrote:
>> Comparing the master with and without your patch, the first, I see is
>> more extensive usage of memory (see complete explains in the attachment):
>>
>> Current master:
>> Batches: 1 Memory Usage: 74513kB
>
>> Patched:
>> Batches: 261 Memory Usage: 527905kB Disk Usage: 4832656kB
>
> Thanks for testing that. I had forgotten to adjust the storage
> location for the hash initial value when I rebased the patch against
> the changes made in 9ca67658d. I've fixed that in the attached patch.
Okay, I passed through the code. It looks good. Hashing expressions are
too simple to give notably impressive results, but it is a step in the
right direction.
I found only one minor issue: an outdated comment (see attachment).
Also, to make a hash calculation as fast as possible, should we add the
call of murmurhash32 as an optional step of ExprState in the
ExecBuildHash32FromAttrs?
--
regards, Andrei Lepikhov