On Tue, Apr 7, 2020 at 7:41 PM David Rowley <dgrowle...@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
