I had been planning to commit v14 this morning but got cold feet with
the BMR-based interface. Heikki didn't like it much, and in the end,
neither did I. I have now removed it, and it seems much better. No
other significant changes, just parameter types and inlining details.
For example:
* read_stream_begin_relation() now takes a Relation, likes its name says
* StartReadBuffers()'s operation takes smgr and optional rel
* ReadBuffer_common() takes smgr and optional rel
ReadBuffer() (which calls ReadBuffer_common() which calls
StartReadBuffer() as before) now shows no regression in a tight loop
over ~1 million already-in-cache pages (something Heikki had observed
before and could only completely fix with a change that affected all
callers). The same test using read_stream.c is still slightly slower,
~1 million pages -in-cache pages 301ms -> 308ms, which seems
acceptable to me and could perhaps be chased down with more study of
inlining/specialisation. As mentioned before, it doesn't seem to be
measurable once you actually do something with the pages.
In some ways BMR was better than the "fake RelationData" concept
(another attempt at wrestling with the relation vs storage duality,
that is, the online vs recovery duality). But in other ways it was
worse: a weird inconsistent mixture of pass-by-pointer and
pass-by-value interfaces that required several code paths to handle it
being only partially initialised, which turned out to be wasted cycles
implicated in regressions, despite which it is not even very nice to
use anyway. I'm sure it could be made to work better, but I'm not yet
sure it's really needed. In later work for recovery I will need to
add a separate constructor read_stream_begin_smgr_something() anyway
for other reasons (multi-relation streaming, different callback) and
perhaps also a separate StartReadBuffersSmgr() if it saves measurable
cycles to strip out branches. Maybe it was all just premature
pessimisation.
So this is the version I'm going to commit shortly, barring objections.
