"Tels" <nospam-pg-ab...@bloodgate.com> writes: > On Wed, December 27, 2017 3:38 pm, Tom Lane wrote: >> Also, I changed PLpgSQL_var.isconst and PLpgSQL_var.notnull from "int" >> to "bool", which is what they should have been all along, and relocated >> them in the PLpgSQL_var struct.
> With a short test program printing out the size of PLpgSQL_var to check, I > always saw 72 bytes, regardless of bool vs. int. So a bool would be 4 > bytes here. Hmm. Seems fairly unlikely, especially given that we typedef bool as char ;-). Which field order were you checking? Are you accounting for alignment padding? By my count, with the existing field order on typical 64-bit hardware, we ought to have PLpgSQL_datum_type dtype; -- 4 bytes [1] int dno; -- 4 bytes char *refname; -- 8 bytes int lineno; -- 4 bytes -- 4 bytes wasted due to padding here PLpgSQL_type *datatype; -- 8 bytes int isconst; -- 4 bytes int notnull; -- 4 bytes PLpgSQL_expr *default_val; -- 8 bytes PLpgSQL_expr *cursor_explicit_expr; -- 8 bytes int cursor_explicit_argrow; -- 4 bytes int cursor_options; -- 4 bytes Datum value; -- 8 bytes bool isnull; -- 1 byte bool freeval; -- 1 byte so at this point we've consumed 74 bytes, but the whole struct has to be aligned on 8-byte boundaries because of the pointers, so sizeof(PLpgSQL_var) ought to be 80 --- and that is what I see here. With the proposed redesign, PLpgSQL_datum_type dtype; -- 4 bytes [1] int dno; -- 4 bytes char *refname; -- 8 bytes int lineno; -- 4 bytes bool isconst; -- 1 byte bool notnull; -- 1 byte -- 2 bytes wasted due to padding here PLpgSQL_type *datatype; -- 8 bytes PLpgSQL_expr *default_val; -- 8 bytes PLpgSQL_expr *cursor_explicit_expr; -- 8 bytes int cursor_explicit_argrow; -- 4 bytes int cursor_options; -- 4 bytes Datum value; -- 8 bytes bool isnull; -- 1 byte bool freeval; -- 1 byte so we've consumed 66 bytes, which rounds up to 72 with the addition of trailing padding. > Googling around, this patch comment from Peter Eisentraut says that "bool" > can be more than one byte, and only "bool8" is really one byte. Even if you're allowing stdbool.h to determine sizeof(bool), I'm pretty sure all Intel-based platforms define that to be 1. > Since I could get the test program to compile against all PG header files > (which is probably me being just being dense...), I used a stripped down > test.c (attached). Hm. I wonder if your <stdbool.h> behaves differently from mine. You might try printing out sizeof(bool) directly to see. > And maybe folding all four bool fields into an "int flags" field with bits > would save space, and not much slower (depending on often how the > different flags are accessed due to the ANDing and ORing ops)? That'd be pretty messy/invasive in terms of the code changes needed, and I don't think it'd save any space once you account for alignment and the fact that my other patch proposes to add another enum at the end of the struct. Also, I'm not exactly convinced that replacing byte sets and tests with bitflag operations would be cheap time-wise. (It would particularly be a mess for isnull, since then there'd be an impedance mismatch with a whole lotta PG APIs that expect null flags to be bools.) regards, tom lane [1] Actually, in principle that enum could be 1, 2, or 4 bytes depending on compiler. But the alignment requirement for dno would mean dtype plus any padding after it would occupy 4 bytes no matter what.