Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 11e178d

Browse filesBrowse files
committed
Inline initial comparisons in TestForOldSnapshot()
Even with old_snapshot_threshold = -1 (which disables the "snapshot too old" feature), performance regressions were seen at moderate to high concurrency. For example, a one-socket, four-core system running 200 connections at saturation could see up to a 2.3% regression, with larger regressions possible on NUMA machines. By inlining the early (smaller, faster) tests in the TestForOldSnapshot() function, the i7 case dropped to a 0.2% regression, which could easily just be noise, and is clearly an improvement. Further testing will show whether more is needed.
1 parent 5b1f9ce commit 11e178d
Copy full SHA for 11e178d

File tree

Expand file treeCollapse file tree

2 files changed

+36
-24
lines changed
Filter options
Expand file treeCollapse file tree

2 files changed

+36
-24
lines changed

‎src/backend/storage/buffer/bufmgr.c

Copy file name to clipboardExpand all lines: src/backend/storage/buffer/bufmgr.c
+5-23Lines changed: 5 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4282,33 +4282,15 @@ IssuePendingWritebacks(WritebackContext *context)
42824282

42834283

42844284
/*
4285-
* Check whether the given snapshot is too old to have safely read the given
4286-
* page from the given table. If so, throw a "snapshot too old" error.
4285+
* Implement slower/larger portions of TestForOldSnapshot
42874286
*
4288-
* This test generally needs to be performed after every BufferGetPage() call
4289-
* that is executed as part of a scan. It is not needed for calls made for
4290-
* modifying the page (for example, to position to the right place to insert a
4291-
* new index tuple or for vacuuming). It may also be omitted where calls to
4292-
* lower-level functions will have already performed the test.
4293-
*
4294-
* Note that a NULL snapshot argument is allowed and causes a fast return
4295-
* without error; this is to support call sites which can be called from
4296-
* either scans or index modification areas.
4297-
*
4298-
* For best performance, keep the tests that are fastest and/or most likely to
4299-
* exclude a page from old snapshot testing near the front.
4287+
* Smaller/faster portions are put inline, but the entire set of logic is too
4288+
* big for that.
43004289
*/
43014290
void
4302-
TestForOldSnapshot(Snapshot snapshot, Relation relation, Page page)
4291+
TestForOldSnapshot_impl(Snapshot snapshot, Relation relation)
43034292
{
4304-
Assert(relation != NULL);
4305-
4306-
if (old_snapshot_threshold >= 0
4307-
&& (snapshot) != NULL
4308-
&& (snapshot)->satisfies == HeapTupleSatisfiesMVCC
4309-
&& !XLogRecPtrIsInvalid((snapshot)->lsn)
4310-
&& PageGetLSN(page) > (snapshot)->lsn
4311-
&& !IsCatalogRelation(relation)
4293+
if (!IsCatalogRelation(relation)
43124294
&& !RelationIsAccessibleInLogicalDecoding(relation)
43134295
&& (snapshot)->whenTaken < GetOldSnapshotThresholdTimestamp())
43144296
ereport(ERROR,

‎src/include/storage/bufmgr.h

Copy file name to clipboardExpand all lines: src/include/storage/bufmgr.h
+31-1Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ extern bool BgBufferSync(struct WritebackContext *wb_context);
239239

240240
extern void AtProcExit_LocalBuffers(void);
241241

242-
extern void TestForOldSnapshot(Snapshot snapshot, Relation relation, Page page);
242+
extern void TestForOldSnapshot_impl(Snapshot snapshot, Relation relation);
243243

244244
/* in freelist.c */
245245
extern BufferAccessStrategy GetAccessStrategy(BufferAccessStrategyType btype);
@@ -257,6 +257,36 @@ extern void FreeAccessStrategy(BufferAccessStrategy strategy);
257257

258258
#ifndef FRONTEND
259259

260+
/*
261+
* Check whether the given snapshot is too old to have safely read the given
262+
* page from the given table. If so, throw a "snapshot too old" error.
263+
*
264+
* This test generally needs to be performed after every BufferGetPage() call
265+
* that is executed as part of a scan. It is not needed for calls made for
266+
* modifying the page (for example, to position to the right place to insert a
267+
* new index tuple or for vacuuming). It may also be omitted where calls to
268+
* lower-level functions will have already performed the test.
269+
*
270+
* Note that a NULL snapshot argument is allowed and causes a fast return
271+
* without error; this is to support call sites which can be called from
272+
* either scans or index modification areas.
273+
*
274+
* For best performance, keep the tests that are fastest and/or most likely to
275+
* exclude a page from old snapshot testing near the front.
276+
*/
277+
static inline void
278+
TestForOldSnapshot(Snapshot snapshot, Relation relation, Page page)
279+
{
280+
Assert(relation != NULL);
281+
282+
if (old_snapshot_threshold >= 0
283+
&& (snapshot) != NULL
284+
&& (snapshot)->satisfies == HeapTupleSatisfiesMVCC
285+
&& !XLogRecPtrIsInvalid((snapshot)->lsn)
286+
&& PageGetLSN(page) > (snapshot)->lsn)
287+
TestForOldSnapshot_impl(snapshot, relation);
288+
}
289+
260290
#endif /* FRONTEND */
261291

262292
#endif /* BUFMGR_H */

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.