> > 0003-hj-refactor-memory-accounting-v4.patch: > > Modify the existing hash join code to work in terms of chunks when > estimating and later tracking memory usage. This is probably more > accurate than the current tuple-based approach, because it tries to > take into account the space used by chunk headers and the wasted space > in chunks. In practice the difference is probably small, but it's > arguably more accurate; I did this because I need chunk-based > accounting the later patches. Also, make HASH_CHUNK_SIZE the actual > size of allocated chunks (ie the header information is included in > that size so we allocate exactly 32KB, not 32KB + a bit, for the > benefit of the dsa allocator which otherwise finishes up allocating > 36KB). > I looked at this patch. I agree that it accounts the memory usage more accurately. Here are few comments.
spaceUsed is defined with comment Size spaceUsed; /* memory space currently used by tuples */ In ExecHashTableCreate(), although the space is allocated for buckets, no tuples are yet inserted, so no space is used by the tuples, so going strictly by the comment, spaceUsed should be 0 in that function. But I think the patch is accounting the spaceUsed more accurately. Without this patch, the actual allocation might cross spaceAllowed without being noticed. With this patch that's not possible. Probably we should change the comment to say memory space currently allocated. However, ExecHashIncreaseNumBatches() may change the number of buckets; the patch does not seem to account for spaceUsed changes because of that. Without this patch ExecHashTableInsert() used to account for the space used by a single tuple inserted. The patch moves this calculation in dense_alloc() and accounts for out-of-bound allocation for larger tuples. That's good. The change in ExecChooseHashTableSize() too looks fine. In ExecHashTableReset(), do we want to update spacePeak while setting spaceUsed. While this patch tracks space usage more accurately, I am afraid we might be overdoing it; a reason why we don't track space usage accurately now. But I think I will leave it to be judged by someone who is more familiar with the code and possibly has historical perspective. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers