pgsql: Remove extra copies of LogwrtResult.

Started by Heikki Linnakangasalmost 14 years ago3 messages
#1Heikki Linnakangas
heikki.linnakangas@iki.fi

Remove extra copies of LogwrtResult.

This simplifies the code a little bit. The new rule is that to update
XLogCtl->LogwrtResult, you must hold both WALWriteLock and info_lck, whereas
before we had two copies, one that was protected by WALWriteLock and another
protected by info_lck. The code that updates them was already holding both
locks, so merging the two is trivial.

The third copy, XLogCtl->Insert.LogwrtResult, was not totally redundant, it
was used in AdvanceXLInsertBuffer to update the backend-local copy, before
acquiring the info_lck to read the up-to-date value. But the value of that
seems dubious; at best it's saving one spinlock acquisition per completed
WAL page, which is not significant compared to all the other work involved.
And in practice, it's probably not saving even that much.

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/7714c6382941383514c0f1954ca831686ac4fcd2

Modified Files
--------------
src/backend/access/transam/xlog.c | 74 +++++++++++--------------------------
1 files changed, 22 insertions(+), 52 deletions(-)

#2Fujii Masao
masao.fujii@gmail.com
In reply to: Heikki Linnakangas (#1)
1 attachment(s)
Re: [COMMITTERS] pgsql: Remove extra copies of LogwrtResult.

On Tue, Mar 6, 2012 at 5:18 PM, Heikki Linnakangas
<heikki.linnakangas@iki.fi> wrote:

Remove extra copies of LogwrtResult.

I found one typo. Attached patch fixes that.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Attachments:

fix_typo.patchtext/x-diff; charset=US-ASCII; name=fix_typo.patchDownload
*** a/src/backend/access/transam/xlog.c
--- b/src/backend/access/transam/xlog.c
***************
*** 6818,6824 **** StartupXLOG(void)
  	else
  	{
  		/*
! 		 * Whenever Write.LogwrtResult points to exactly the end of a page,
  		 * Write.curridx must point to the *next* page (see XLogWrite()).
  		 *
  		 * Note: it might seem we should do AdvanceXLInsertBuffer() here, but
--- 6818,6824 ----
  	else
  	{
  		/*
! 		 * Whenever LogwrtResult points to exactly the end of a page,
  		 * Write.curridx must point to the *next* page (see XLogWrite()).
  		 *
  		 * Note: it might seem we should do AdvanceXLInsertBuffer() here, but
#3Robert Haas
robertmhaas@gmail.com
In reply to: Fujii Masao (#2)
Re: [COMMITTERS] pgsql: Remove extra copies of LogwrtResult.

On Tue, Mar 6, 2012 at 7:06 AM, Fujii Masao <masao.fujii@gmail.com> wrote:

On Tue, Mar 6, 2012 at 5:18 PM, Heikki Linnakangas
<heikki.linnakangas@iki.fi> wrote:

Remove extra copies of LogwrtResult.

I found one typo. Attached patch fixes that.

Committed.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company