Robert Haas <robertmhaas@gmail.com> writes:
> I don't understand why you think this would create non-trivial
> portability issues.
The patch as submitted breaks entirely on platforms without pread/pwrite.
Yes, we can add a configure test and some shim functions to fix that,
but the argument that it makes the code shorter will get a lot weaker
once we do.
I agree that adding such functions is pretty trivial, but there are
reasons to think there are other hazards that are less trivial:
First, a self-contained shim function will necessarily do an lseek every
time, which means performance will get *worse* not better on non-pread
platforms. And yes, the existing logic to avoid lseeks fires often enough
to be worthwhile, particularly in seqscans.
Second, I wonder whether this will break any kernel's readahead detection.
I wouldn't be too surprised if successive reads (not preads) without
intervening lseeks are needed to trigger readahead on at least some
platforms. So there's a potential, both on platforms with pread and those
without, for this to completely destroy seqscan performance, with
penalties very far exceeding what we might save by avoiding some kernel
calls.
I'd be more excited about this if the claimed improvement were more than
1.5%, but you know as well as I do that that's barely above the noise
floor for most performance measurements. I'm left wondering why bother,
and why take any risk of de-optimizing on some platforms.
regards, tom lane
