NotificationsYou must be signed in to change notification settings
Fork5
Star26

Commita6f6b78

committed

Use static inline function for BufferGetPage()

I was initially concerned that the some of the hundreds ofreferences to BufferGetPage() where the literalBGP_NO_SNAPSHOT_TEST were passed might not optimize as well as amacro, leading to some hard-to-find performance regressions incorner cases. Inspection of disassembled code has shown identicalcode at all inspected locations, and the size difference doesn'tamount to even one byte per such call. So make it readable.Per gripes from Álvaro Herrera and Tom Lane

1 parent80647bf commita6f6b78Copy full SHA for a6f6b78

File tree

2 files changed

+25

-29

lines changed

src
- backend/storage/buffer
  - bufmgr.c
- include/storage
  - bufmgr.h

2 files changed

+25

-29

lines changed

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

Lines changed: 1 addition & 3 deletions

Original file line number	Diff line number	Diff line change
`@@ -4295,7 +4295,7 @@ IssuePendingWritebacks(WritebackContext *context)`
`4295`	`4295`	`* For best performance, keep the tests that are fastest and/or most likely to`
`4296`	`4296`	`* exclude a page from old snapshot testing near the front.`
`4297`	`4297`	`*/`
`4298`		`-externPage`
	`4298`	`+externvoid`
`4299`	`4299`	`TestForOldSnapshot(Snapshotsnapshot,Relationrelation,Pagepage)`
`4300`	`4300`	`{`
`4301`	`4301`	`Assert(relation!=NULL);`
`@@ -4311,6 +4311,4 @@ TestForOldSnapshot(Snapshot snapshot, Relation relation, Page page)`
`4311`	`4311`	`ereport(ERROR,`
`4312`	`4312`	`(errcode(ERRCODE_SNAPSHOT_TOO_OLD),`
`4313`	`4313`	`errmsg("snapshot too old")));`
`4314`		`-`
`4315`		`-returnpage;`
`4316`	`4314`	`}`

`‎src/include/storage/bufmgr.h`

Lines changed: 24 additions & 26 deletions

Original file line number	Diff line number	Diff line change
`@@ -177,31 +177,6 @@ extern PGDLLIMPORT int32 *LocalRefCount;`
`177`	`177`	`(Size)BLCKSZ \`
`178`	`178`	`)`
`179`	`179`
`180`		`-/*`
`181`		`- * BufferGetPage`
`182`		`- *Returns the page associated with a buffer.`
`183`		`- *`
`184`		`- * agetest will normally be a literal, so use a macro at the outer level to`
`185`		`- * give the compiler a chance to optimize away the runtime code to check it.`
`186`		`- *`
`187`		`- * TestForOldSnapshot(), if it doesn't throw an error, will return the page`
`188`		`- * argument it is passed, so the same result will go back to this macro's`
`189`		`- * caller for either agetest value; it is a matter of whether to call the`
`190`		`- * function to perform the test. For call sites where the check is not needed`
`191`		`- * (which is the vast majority of them), the snapshot and relation parameters`
`192`		`- * can, and generally should, be NULL.`
`193`		`- */`
`194`		`-#defineBufferGetPage(buffer,snapshot,relation,agetest) \`
`195`		`-( \`
`196`		`-( \`
`197`		`-AssertMacro((agetest) == BGP_NO_SNAPSHOT_TEST \|\| (agetest) == BGP_TEST_FOR_OLD_SNAPSHOT), \`
`198`		`-((agetest) == BGP_NO_SNAPSHOT_TEST) \`
`199`		`-) ? \`
`200`		`-((Page)BufferGetBlock(buffer)) \`
`201`		`-: \`
`202`		`-(TestForOldSnapshot(snapshot, relation, (Page)BufferGetBlock(buffer))) \`
`203`		`-)`
`204`		`-`
`205`	`180`	`/*`
`206`	`181`	`* prototypes for functions in bufmgr.c`
`207`	`182`	`*/`
`@@ -268,10 +243,33 @@ extern bool BgBufferSync(struct WritebackContext *wb_context);`
`268`	`243`
`269`	`244`	`externvoidAtProcExit_LocalBuffers(void);`
`270`	`245`
`271`		`-externPageTestForOldSnapshot(Snapshotsnapshot,Relationrelation,Pagepage);`
	`246`	`+externvoidTestForOldSnapshot(Snapshotsnapshot,Relationrelation,Pagepage);`
`272`	`247`
`273`	`248`	`/* in freelist.c */`
`274`	`249`	`externBufferAccessStrategyGetAccessStrategy(BufferAccessStrategyTypebtype);`
`275`	`250`	`externvoidFreeAccessStrategy(BufferAccessStrategystrategy);`
`276`	`251`
	`252`	`+`
	`253`	`+/* inline functions */`
	`254`	`+`
	`255`	`+/*`
	`256`	`+ * BufferGetPage`
	`257`	`+ *Returns the page associated with a buffer.`
	`258`	`+ *`
	`259`	`+ * For call sites where the check is not needed (which is the vast majority of`
	`260`	`+ * them), the snapshot and relation parameters can, and generally should, be`
	`261`	`+ * NULL.`
	`262`	`+ */`
	`263`	`+staticinlinePage`
	`264`	`+BufferGetPage(Bufferbuffer,Snapshotsnapshot,Relationrelation,`
	`265`	`+BufferGetPageAgeTestagetest)`
	`266`	`+{`
	`267`	`+Pagepage= (Page)BufferGetBlock(buffer);`
	`268`	`+`
	`269`	`+if (agetest==BGP_TEST_FOR_OLD_SNAPSHOT)`
	`270`	`+TestForOldSnapshot(snapshot,relation,page);`
	`271`	`+`
	`272`	`+returnpage;`
	`273`	`+}`
	`274`	`+`
`277`	`275`	`#endif`

0 commit comments

Comments

(0)

Movatterモバイル変換

Navigation Menu

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Commita6f6b78

File tree

2 files changed

2 files changed

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

`‎src/include/storage/bufmgr.h`

0 commit comments