ITAGAKI Takahiro <itagaki.takahiro@lab.ntt.co.jp> writes:
> Attached is a patch that removes undesired paddings from b-tree indexes.
This seems extremely invasive for a relatively small gain :-(
The example you cite of an int4 index on a MAXALIGN-8 machine is
by far the best case, and in many cases there wouldn't be anything
bought by the extra complexity.
Aside from the code uglification, I'm concerned about the performance
implications of, for example, changing PageAddItem so that it has to
cope with non-compile-time-constant alignment calculations. That adds
overhead for a lot of places that have nothing to do with btrees.
I'm also fairly sure that you've broken xlog recovery for btrees,
eg, btree_xlog_newroot has no way to set up the btpo_align field.
(Personally I'd try to get rid of btpo_align, not fix xlog to
maintain it.)
You could simplify some things by continuing to use MAXALIGN in places
that are just doing freespace calculations and not actually placing a
tuple on a page. This would waste at worst 4 bytes per page so it does
not seem worthwhile to be more precise.
Another thing that would be interesting to think about is adopting the
convention that lp_len should always be the length with alignment
padding included. This'd eliminate the need to do repetitive alignment
calculations in some places like PageRepairFragmentation and
PageIndexMultiDelete, and thus avoid the need to pass alignment
information to them at all. I can't at the moment see a reason why
we need to remember the tuple length to exact byte precision.
(This would however require that all tuples on a page have the *same*
alignment requirement, else compaction might place some at unsuitable
locations. The code you put in index_form_tuple to make the alignment
variable depending on presence of nulls does not seem like a good idea
to me in any case.)
I'd also advise not making unnecessary changes in function names;
adding Aligned to those function names isn't doing anything to improve
readability of the code.
regards, tom lane
