At 2013-07-12 16:25:14 -0700, jeff.janes@gmail.com wrote:
>
> I think the reviewer of a performance patch should do some independent
> testing of the performance, to replicate the author's numbers; and
> hopefully with a few different scenarios.
You're quite right. I apologise for being lazy; doubly so because I
can't actually see any difference while running the test case with
the patches applied.
unpatched: before: 1629.831391, 1559.793758, 1498.765018, 1639.384038 during: 37.434492, 37.044989,
37.112422, 36.950895 after : 46.591688, 46.341256, 46.042169, 46.260684
patched: before: 1813.091975, 1798.923524, 1629.301356, 1606.849033 during: 37.344987, 37.207359, 37.406788,
37.316925 after : 46.657747, 46.537420, 46.746377, 46.577052
("before" is before starting session 2; "during" is after session 2
inserts, but before it commits; "after" is after session 2 issues a
rollback.)
The timings above are with both xid_in_snapshot_cache.v1.patch and
cache_TransactionIdInProgress.v2.patch applied, but the numbers are
not noticeably different with only the first patch applied. After I
"vacuum plan", the timings in both cases return to normal.
In a quick test with gdb (and also in perf report output), I didn't see
the following block in procarray.c being entered at all:
+ if (max_prepared_xacts == 0 && pgprocno >= 0 &&
+ (TransactionIdEquals(xid, pxid) || TransactionIdEquals(xid, cxid)))
+ { …
I'll keep looking, but comments are welcome. I'm setting this back to
"Needs Review" in the meantime.
-- Abhijit