Re: Signed vs Unsigned (take 2) (src/backend/storage/ipc/procarray.c)

Em qui., 15 de jul. de 2021 às 08:38, Aleksander Alekseev <
aleksander(at)timescale(dot)com> escreveu:

> Hi hackers,
>
> >> Patch attached.
> > Added to next CF (https://commitfest.postgresql.org/33/3169/)
>
> Hi Aleksander, thanks for taking a look at this.

> The proposed code casts `const` variables to non-`const`. I'm surprised
> MSVC misses it.
>
I lost where. Can you show me?

> Also, there were some issues with the code formatting. The corrected patch
> is attached.
>
Sorry, thanks for correcting.

> The patch is listed under the "Performance" topic on CF. However, I can't
> verify any changes in the performance because there were no benchmarks
> attached that I could reproduce. By looking at the code and the first
> message in the thread, I assume this is in fact a refactoring.
>
My mistake, a serious fault.
But the benchmark came from:
pgbench -i -p 5432 -d postgres
pgbench -c 50 -T 300 -S -n

>
> Personally I don't believe that changes like:
>
> - for (int i = 0; i < nxids; i++)
> + int i;
> + for (i = 0; i < nxids; i++)
>
Yeah, it seems to me that this style will be consolidated in Postgres 'for
(int i = 0;'.

>
> .. or:
>
> - for (int index = myoff; index < arrayP->numProcs; index++)
> + numProcs = arrayP->numProcs;
> + for (index = myoff; index < numProcs; index++)
>
The rationale here is to cache arrayP->numProcs to local variable, which
improves performance.

>
> ... are of any value, but other changes may be. I choose to keep the patch
> as-is except for the named defects and let the committer decide which
> changes, if any, are worth committing.
>
> I'm updating the status to "Ready for Committer".
>
Thank you.

regards,
Ranier Vilela