On Mon, Jun 12, 2023 at 8:53 PM David Steele <da...@pgmasters.net> wrote:
> + if (strcmp(endptr, "kB") == 0)
>
> Why kB here instead of KB to match MB, GB, TB below?
Those are SI prefixes[1], and we use kB elsewhere too. ("K" was used
for kelvins, so they went with "k" for kilo. Obviously these aren't
fully SI, because B is supposed to mean bel. A gigabel would be
pretty loud... more than "sufficient power to create a black hole"[2],
hehe.)
> + int64 relseg_size; /* blocks per segment of large
> relation */
>
> This will require PG_CONTROL_VERSION to be bumped -- but you are
> probably waiting until commit time to avoid annoying conflicts, though I
> don't think it is as likely as with CATALOG_VERSION_NO.
Oh yeah, thanks.
> > Another
> > idea would be to make it static in md.c and call smgrsetsegmentsize(),
> > or something like that. That could be a nice place to compute the
> > "shift" value up front, instead of computing it each time in
> > blockno_to_segno(), but that's probably not worth bothering with (?).
> > BSR/LZCNT/CLZ instructions are pretty fast on modern chips. That's
> > about the only place where someone could say that this change makes
> > things worse for people not interested in the new feature, so I was
> > careful to get rid of / and % operations with no-longer-constant RHS.
>
> Right -- not sure we should be troubling ourselves with trying to
> optimize away ops that are very fast, unless they are computed trillions
> of times.
This obviously has some things in common with David Christensen's
nearby patch for block sizes[3], and we should be shifting and masking
there too if that route is taken (as opposed to a specialise-the-code
route or somethign else). My binary-log trick is probably a little
too cute though... I should probably just go and set a shift variable.
Thanks for looking!
[1] https://en.wikipedia.org/wiki/Metric_prefix
[2] https://en.wiktionary.org/wiki/gigabel
[3]
https://www.postgresql.org/message-id/flat/CAOxo6XKx7DyDgBkWwPfnGSXQYNLpNrSWtYnK6-1u%2BQHUwRa1Gg%40mail.gmail.com
