On Tue, Apr 7, 2020 at 7:41 PM David Rowley <dgrowleyml@gmail.com> wrote:
> I've attached an updated patch. It includes the modifications
> mentioned above to pre-check for a power of 2 number with the bit
> masking hack mentioned above. I also renamed the functions to be more
> aligned to the other functions in pg_bitutils.h I'm not convinced
> pg_ceil_log2_* needs the word "ceil" in there.
>
> I dropped the part of the patch that was changing longs to ints of a
> known size. I went on and did some additional conversion in the 0003
> patch. There are more laying around the code base, but I ended up
> finding a bit to fix up than i had thought I would. e.g. various
> places that repalloc() inside a loop that is multiplying the
> allocation size by 2 each time. The repalloc should be done at the
> end, not during the loop. I thought I might come back to those some
> time in the future.
>
> Is anyone able to have a look at this?
>
> David
Hi David,
Overall looks good to me. Just a couple things I see:
It seems _hash_log2 is still in the tree, but has no callers?
- max_size = 8; /* semi-arbitrary small power of 2 */
- while (max_size < min_size + LIST_HEADER_OVERHEAD)
- max_size *= 2;
+ max_size = pg_nextpower2_32(Max(8, min_size + LIST_HEADER_OVERHEAD));
Minor nit: We might want to keep the comment that the number is
"semi-arbitrary" here as well.
- 'pg_waldump', 'scripts');
+ 'pg_validatebackup', 'pg_waldump', 'scripts');
This seems like a separate concern?
--
John Naylor https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
