>From 04004c045204e1dcf1ce0125a890423bbefe9ae4 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Wed, 18 Dec 2013 18:26:57 -0300 Subject: [PATCH] Optimize updating a row that's locked by same xact 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 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 an application does SELECT FOR UPDATE followed by an UPDATE that does not modify columns of the key, we would represent this as a multixact. Optimize the case so that only the strongest of both locks/updates is represented in Xmax. This can save some operations from acquiring MultiXacts, which is 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. Andres Freund, tweaked by Álvaro Herrera --- src/backend/access/heap/heapam.c | 45 +++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 70748e5..6b78783 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -4541,6 +4541,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) { @@ -4705,37 +4710,30 @@ l5: status = MultiXactStatusNoKeyUpdate; } - new_status = get_mxact_status_for_lock(mode, is_update); - /* - * 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 from the same transaction as the + * existing lock, there's an additional optimization: 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. + * The old xmax cannot possibly be an update, since otherwise this + * row version would have been invisible. */ - 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; + + old_infomask |= HEAP_XMAX_INVALID; + goto l5; } + + new_status = get_mxact_status_for_lock(mode, is_update); new_xmax = MultiXactIdCreate(xmax, status, add_to_xmax, new_status); GetMultiXactIdHintBits(new_xmax, &new_infomask, &new_infomask2); } -- 1.7.10.4