From 13aa6244313df2315b0952a9e7f6e68bb09c9d98 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Thu, 19 Dec 2013 16:39:59 -0300
Subject: [PATCH] Optimize updating a row that's locked by same xid
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Updating or locking a row that was already locked by the same
transaction under the same Xid caused a MultiXact to be created; but
this is unnecessary, because there's no usefulness in being able to
differentiate two locks by the same transaction. In particular, if a
transaction executed SELECT FOR UPDATE followed by an UPDATE that didn't
modify columns of the key, we would dutifully represent the resulting
combination as a multixact -- even though a single key-update is
sufficient.
Optimize the case so that only the strongest of both locks/updates is
represented in Xmax. This can save some Xmax's from becoming
MultiXacts, which can be a significant optimization.
This missed optimization opportunity was spotted by Andres Freund while
investigating a bug reported by Oliver Seemann in message
CANCipfpfzoYnOz5jj=UZ70_R=CwDHv36dqWSpwsi27vpm1z5sA@mail.gmail.com
and also directly as a performance regression reported by Dong Ye in
message
d54b8387.000012d8.00000010@YED-DEVD1.vmware.com
Reportedly, this patch fixes the performance regression.
Since the missing optimization was reported as a significant performance
regression from 9.2, backpatch to 9.3.
Andres Freund, tweaked by Álvaro Herrera
---
src/backend/access/heap/heapam.c | 70 ++++++++++++++++++--------------
1 file changed, 39 insertions(+), 31 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index db683b12179..4dfc8b1de50 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -4705,6 +4705,8 @@ compute_new_xmax_infomask(TransactionId xmax, uint16 old_infomask,
uint16 new_infomask,
new_infomask2;
+ Assert(TransactionIdIsCurrentTransactionId(add_to_xmax));
+
l5:
new_infomask = 0;
new_infomask2 = 0;
@@ -4712,6 +4714,11 @@ l5:
{
/*
* No previous locker; we just insert our own TransactionId.
+ *
+ * Note that it's critical that this case be the first one checked,
+ * because there are several blocks below that come back to this one
+ * to implement certain optimizations; old_infomask might contain
+ * other dirty bits in those cases, but we don't really care.
*/
if (is_update)
{
@@ -4837,21 +4844,22 @@ l5:
* create a new MultiXactId that includes both the old locker or
* updater and our own TransactionId.
*/
- MultiXactStatus status;
MultiXactStatus new_status;
+ MultiXactStatus old_status;
+ LockTupleMode old_mode;
if (HEAP_XMAX_IS_LOCKED_ONLY(old_infomask))
{
if (HEAP_XMAX_IS_KEYSHR_LOCKED(old_infomask))
- status = MultiXactStatusForKeyShare;
+ old_status = MultiXactStatusForKeyShare;
else if (HEAP_XMAX_IS_SHR_LOCKED(old_infomask))
- status = MultiXactStatusForShare;
+ old_status = MultiXactStatusForShare;
else if (HEAP_XMAX_IS_EXCL_LOCKED(old_infomask))
{
if (old_infomask2 & HEAP_KEYS_UPDATED)
- status = MultiXactStatusForUpdate;
+ old_status = MultiXactStatusForUpdate;
else
- status = MultiXactStatusForNoKeyUpdate;
+ old_status = MultiXactStatusForNoKeyUpdate;
}
else
{
@@ -4871,43 +4879,43 @@ l5:
{
/* it's an update, but which kind? */
if (old_infomask2 & HEAP_KEYS_UPDATED)
- status = MultiXactStatusUpdate;
+ old_status = MultiXactStatusUpdate;
else
- status = MultiXactStatusNoKeyUpdate;
+ old_status = MultiXactStatusNoKeyUpdate;
}
- new_status = get_mxact_status_for_lock(mode, is_update);
+ old_mode = TUPLOCK_from_mxstatus(old_status);
/*
- * If the existing lock mode is identical to or weaker than the new
- * one, we can act as though there is no existing lock, so set
- * XMAX_INVALID and restart.
+ * If the lock to be acquired is for the same TransactionId as the
+ * existing lock, there's an optimization possible: consider only the
+ * strongest of both locks as the only one present, and restart.
*/
if (xmax == add_to_xmax)
{
- LockTupleMode old_mode = TUPLOCK_from_mxstatus(status);
- bool old_isupd = ISUPDATE_from_mxstatus(status);
-
/*
- * We can do this if the new LockTupleMode is higher or equal than
- * the old one; and if there was previously an update, we need an
- * update, but if there wasn't, then we can accept there not being
- * one.
+ * Note that it's not possible for the original tuple to be updated:
+ * we wouldn't be here because the tuple would have been invisible and
+ * we wouldn't try to update it. As a subtlety, this code can also
+ * run when traversing an update chain to lock future versions of a
+ * tuple. But we wouldn't be here either, because the add_to_xmax
+ * would be different from the original updater.
*/
- if ((mode >= old_mode) && (is_update || !old_isupd))
- {
- /*
- * Note that the infomask might contain some other dirty bits.
- * However, since the new infomask is reset to zero, we only
- * set what's minimally necessary, and that the case that
- * checks HEAP_XMAX_INVALID is the very first above, there is
- * no need for extra cleanup of the infomask here.
- */
- old_infomask |= HEAP_XMAX_INVALID;
- goto l5;
- }
+ Assert(HEAP_XMAX_IS_LOCKED_ONLY(old_infomask));
+
+ /* acquire the strongest of both */
+ if (mode < old_mode)
+ mode = old_mode;
+ /* mustn't touch is_update */
+
+ old_infomask |= HEAP_XMAX_INVALID;
+ goto l5;
}
- new_xmax = MultiXactIdCreate(xmax, status, add_to_xmax, new_status);
+
+ /* otherwise, just fall back to creating a new multixact */
+ new_status = get_mxact_status_for_lock(mode, is_update);
+ new_xmax = MultiXactIdCreate(xmax, old_status,
+ add_to_xmax, new_status);
GetMultiXactIdHintBits(new_xmax, &new_infomask, &new_infomask2);
}
else if (!HEAP_XMAX_IS_LOCKED_ONLY(old_infomask) &&
--
GitLab