On Sat, 1 Aug 2020 at 12:17, Thomas Munro <thomas.munro@gmail.com> wrote:
>
> On Sat, Aug 1, 2020 at 7:22 AM James Coleman <jtc331@gmail.com> wrote:
> > [v2 patch set]
>
> I ran it through pgindent which insisted on adding some newlines, I
> manually replaced some spaces with tabs to match nearby lines, I added
> some asterisks in your example function prototypes where <element> is
> returned because they seemed to be missing, and I pushed this.
> Thanks!
I was just reading over this and wondered about the following:
+ * The element type is required to contain a "uint32 status" member.
I see that PagetableEntry does not follow this and I also didn't
follow it when writing the Result Cache patch in [1]. I managed to
shrink the struct I was using for the hash table by 4 bytes by using a
char instead of an int. That sounds like a small amount of memory, but
it did result in much better cache hit ratios in the patch
Maybe it would be better just to get rid of the enum and just #define
the values. It seems unlikely that we're every going to need many more
states than what are there already, let along more than, say 127 of
them. It does look like manifest_file could be shrunk down a bit too
by making the status field a char.
[1] https://www.postgresql.org/message-id/flat/CAApHDvrPcQyQdWERGYWx8J+2DLUNgXu+fOSbQ1UscxrunyXyrQ@mail.gmail.com