Demote some sanity checks in BufferIsValid() to assertions.

Testing reveals that this macro is a hot-spot for index-only-scans.
Per discussion with Tom Lane.

diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h
index b8fc87ec57f9a8da601a7ef88f04d86f01adb79d..82b43006f3e0be0e5dd724de943c48e270edc4db 100644 (file)
--- a/src/include/storage/bufmgr.h
+++ b/src/include/storage/bufmgr.h
@@ -78,20 +78,24 @@ extern PGDLLIMPORT int32 *LocalRefCount;
  *             True iff the given buffer number is valid (either as a shared
  *             or local buffer).
  *
- * This is not quite the inverse of the BufferIsInvalid() macro, since this
- * adds sanity rangechecks on the buffer number.
- *
  * Note: For a long time this was defined the same as BufferIsPinned,
  * that is it would say False if you didn't hold a pin on the buffer.
  * I believe this was bogus and served only to mask logic errors.
  * Code should always know whether it has a buffer reference,
  * independently of the pin state.
+ *
+ * Note: For a further long time this was not quite the inverse of the
+ * BufferIsInvalid() macro, in that it also did sanity checks to verify
+ * that the buffer number was in range.  Most likely, this macro was
+ * originally intended only to be used in assertions, but its use has
+ * since expanded quite a bit, and the overhead of making those checks
+ * even in non-assert-enabled builds can be significant.  Thus, we've
+ * now demoted the range checks to assertions within the macro itself.
  */
 #define BufferIsValid(bufnum) \
 ( \
-       (bufnum) != InvalidBuffer && \
-       (bufnum) >= -NLocBuffer && \
-       (bufnum) <= NBuffers \
+       AssertMacro((bufnum) <= NBuffers && (bufnum) >= -NLocBuffer), \
+       (bufnum) != InvalidBuffer  \
 )
 
 /*