>From e0520c881bea653e33d9b5ef5bff065840ebacb5 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Wed, 18 Dec 2013 21:51:41 -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 | 64 ++++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 30 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 70748e5..023db6a 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) { @@ -4659,6 +4664,33 @@ l5: new_xmax = MultiXactIdCreate(xmax, status, add_to_xmax, new_status); GetMultiXactIdHintBits(new_xmax, &new_infomask, &new_infomask2); } + else if (xmax == add_to_xmax && TransactionIdIsCurrentTransactionId(xmax)) + + { + LockTupleMode old_mode = TUPLOCK_from_mxstatus(status); + + /* + * 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. + * + * 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. + */ + Assert(HEAP_XMAX_IS_LOCKED_ONLY(old_infomask)); + + /* acquire the strongest of both */ + if (mode < old_mode) + mode = old_mode; + /* don't need to touch is_update */ + + old_infomask |= HEAP_XMAX_INVALID; + goto l5; + } else if (TransactionIdIsInProgress(xmax)) { /* @@ -4669,6 +4701,8 @@ l5: MultiXactStatus status; MultiXactStatus new_status; + Assert(xmax != add_to_xmax); + if (HEAP_XMAX_IS_LOCKED_ONLY(old_infomask)) { if (HEAP_XMAX_IS_KEYSHR_LOCKED(old_infomask)) @@ -4706,36 +4740,6 @@ l5: } 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 (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. - */ - 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; - } - } new_xmax = MultiXactIdCreate(xmax, status, add_to_xmax, new_status); GetMultiXactIdHintBits(new_xmax, &new_infomask, &new_infomask2); } -- 1.7.10.4