Em qui., 15 de jul. de 2021 às 10:04, Ranier Vilela <ranier...@gmail.com>
escreveu:
> Em qui., 15 de jul. de 2021 às 10:01, Aleksander Alekseev <
> aleksan...@timescale.com> escreveu:
>
>> Thanks, David.
>>
>> > I lost where. Can you show me?
>>
>> See the attached warnings.txt.
>>
> Thank you.
>
>
>>
>> > But the benchmark came from:
>> > pgbench -i -p 5432 -d postgres
>> > pgbench -c 50 -T 300 -S -n
>>
>> I'm afraid this tells nothing unless you also provide the
>> configuration files and the hardware description, and also some
>> information on how you checked that there is no performance
>> degradation on all the other supported platforms and possible
>> configurations.
>
>
>
>> Benchmarking is a very complicated topic - trust me,
>> been there!
>>
> Absolutely.
>
>
>>
>> It would be better to submit two separate patches, the one that
>> addresses Size_t and another that addresses shadowing. Refactorings
>> only, nothing else.
>>
>> Regarding the code formatting, please see src/tools/pgindent.
>>
> I will try.
>
Here are the two patches.
As suggested, reclassified as refactoring only.
regards,
Ranier Vilela
From 57eaa5fa92489b217239ec44e2ccf83556a1da4f Mon Sep 17 00:00:00 2001
From: Ranier Vilela <ranier...@gmail.com>
Date: Thu, 15 Jul 2021 21:36:21 -0300
Subject: [PATCH] Promove unshadowing of two variables PGPROC type.
ProcArrayGroupClearXid function has a parameter already named *proc*,
When declaring a local variable with the same name, people may get confused when using them.
Better to rename local variables to nextproc and maintain readability.
No need to backpatch, this patch is classified as refactoring only.
---
src/backend/storage/ipc/procarray.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 4c91e721d0..286132c696 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -829,12 +829,12 @@ ProcArrayGroupClearXid(PGPROC *proc, TransactionId latestXid)
/* Walk the list and clear all XIDs. */
while (nextidx != INVALID_PGPROCNO)
{
- PGPROC *proc = &allProcs[nextidx];
+ PGPROC *nextproc = &allProcs[nextidx];
- ProcArrayEndTransactionInternal(proc, proc->procArrayGroupMemberXid);
+ ProcArrayEndTransactionInternal(nextproc, proc->procArrayGroupMemberXid);
/* Move to next proc in list. */
- nextidx = pg_atomic_read_u32(&proc->procArrayGroupNext);
+ nextidx = pg_atomic_read_u32(&nextproc->procArrayGroupNext);
}
/* We're done with the lock now. */
@@ -849,18 +849,18 @@ ProcArrayGroupClearXid(PGPROC *proc, TransactionId latestXid)
*/
while (wakeidx != INVALID_PGPROCNO)
{
- PGPROC *proc = &allProcs[wakeidx];
+ PGPROC *nextproc = &allProcs[wakeidx];
- wakeidx = pg_atomic_read_u32(&proc->procArrayGroupNext);
- pg_atomic_write_u32(&proc->procArrayGroupNext, INVALID_PGPROCNO);
+ wakeidx = pg_atomic_read_u32(&nextproc->procArrayGroupNext);
+ pg_atomic_write_u32(&nextproc->procArrayGroupNext, INVALID_PGPROCNO);
/* ensure all previous writes are visible before follower continues. */
pg_write_barrier();
- proc->procArrayGroupMember = false;
+ nextproc->procArrayGroupMember = false;
- if (proc != MyProc)
- PGSemaphoreUnlock(proc->sem);
+ if (nextproc != MyProc)
+ PGSemaphoreUnlock(nextproc->sem);
}
}
--
2.25.1
From fafb23a62835f7d35bd3f4642b06253cf16cbfca Mon Sep 17 00:00:00 2001
From: Ranier Vilela <ranier...@gmail.com>
Date: Thu, 15 Jul 2021 21:40:20 -0300
Subject: [PATCH] Reduce Wsign-compare warnings from clang-12 compiler.
All size_t variables declared in procarray.c, are true ints.
Lets promove a changes for int, reducing warnings like
"comparison of integers of different signs".
No need to backpatch, why this patch is classified as
refactoring only.
---
src/backend/storage/ipc/procarray.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 286132c696..efb489bc5d 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -703,7 +703,7 @@ ProcArrayEndTransaction(PGPROC *proc, TransactionId latestXid)
static inline void
ProcArrayEndTransactionInternal(PGPROC *proc, TransactionId latestXid)
{
- size_t pgxactoff = proc->pgxactoff;
+ int pgxactoff = proc->pgxactoff;
/*
* Note: we need exclusive lock here because we're going to change other
@@ -875,7 +875,7 @@ ProcArrayGroupClearXid(PGPROC *proc, TransactionId latestXid)
void
ProcArrayClearTransaction(PGPROC *proc)
{
- size_t pgxactoff;
+ int pgxactoff;
/*
* Currently we need to lock ProcArrayLock exclusively here, as we
@@ -1355,7 +1355,7 @@ TransactionIdIsInProgress(TransactionId xid)
TransactionId topxid;
TransactionId latestCompletedXid;
int mypgxactoff;
- size_t numProcs;
+ int numProcs;
int j;
/*
@@ -1432,7 +1432,7 @@ TransactionIdIsInProgress(TransactionId xid)
/* No shortcuts, gotta grovel through the array */
mypgxactoff = MyProc->pgxactoff;
numProcs = arrayP->numProcs;
- for (size_t pgxactoff = 0; pgxactoff < numProcs; pgxactoff++)
+ for (int pgxactoff = 0; pgxactoff < numProcs; pgxactoff++)
{
int pgprocno;
PGPROC *proc;
@@ -2167,7 +2167,7 @@ GetSnapshotData(Snapshot snapshot)
TransactionId *other_xids = ProcGlobal->xids;
TransactionId xmin;
TransactionId xmax;
- size_t count = 0;
+ int count = 0;
int subcount = 0;
bool suboverflowed = false;
FullTransactionId latest_completed;
@@ -2249,7 +2249,7 @@ GetSnapshotData(Snapshot snapshot)
if (!snapshot->takenDuringRecovery)
{
- size_t numProcs = arrayP->numProcs;
+ int numProcs = arrayP->numProcs;
TransactionId *xip = snapshot->xip;
int *pgprocnos = arrayP->pgprocnos;
XidCacheStatus *subxidStates = ProcGlobal->subxidStates;
@@ -2259,7 +2259,7 @@ GetSnapshotData(Snapshot snapshot)
* First collect set of pgxactoff/xids that need to be included in the
* snapshot.
*/
- for (size_t pgxactoff = 0; pgxactoff < numProcs; pgxactoff++)
+ for (int pgxactoff = 0; pgxactoff < numProcs; pgxactoff++)
{
/* Fetch xid just once - see GetNewTransactionId */
TransactionId xid = UINT32_ACCESS_ONCE(other_xids[pgxactoff]);
--
2.25.1
