Horton

Thread

Commits

  1. Get rid of our dependency on type "long" for memory size calculations.

  2. Use "ssize_t" not "long" in max_stack_depth-related code.

  3. Avoid integer overflow while testing wal_skip_threshold condition.

  1. Increase of maintenance_work_mem limit in 64-bit Windows

    Vladlen Popolitov <v.popolitov@postgrespro.ru> — 2024-09-19T13:55:42Z

    
Hello!
Currently PostgreSQL built on 64-bit Windows has 2Gb limit for
GUC variables due to sizeof(long)==4 used by Windows compilers.
Technically 64-bit addressing for maintenance_work_mem is possible,
but code base historically uses variables and constants of type "long",
when process maintenance_work_mem value.
Modern vector indexes like pgvector or pgvectorscale require as much
as possible maintenance_work_mem, 2 Gb limit  dramatically decrease
build index performace making impossible to build indexes for large
datasets.
​​​ The proposed patch fixes all appearences of "long" variables and constants
that can affect maintenance_work_mem (hash index, vacuum, planner only
affected, gin, gist, brin, bloom, btree indexes process value
correctly).
Constant MAX_SIZE_T_KILOBYTES added as upper limit for GUC variables
that depend on size_t only (currently only maintenance_work_mem).
Other GUC variables could use this constant after fixing "long" type
dependence.
This patch tested on
a) Windows 10 64-bit AMD64, compiled by msvc-19.37.32822
b) linux gcc (Debian 12.2.0-14) AMD64
All tests are passed.

Best regards

Vladlen Popolitov
postgrespro.com

 

  2. Re: Increase of maintenance_work_mem limit in 64-bit Windows

    David Rowley <dgrowleyml@gmail.com> — 2024-09-23T01:28:47Z

    On Fri, 20 Sept 2024 at 01:55, Пополитов Владлен
<v.popolitov@postgrespro.ru> wrote:
> Currently PostgreSQL built on 64-bit Windows has 2Gb limit for
> GUC variables due to sizeof(long)==4 used by Windows compilers.
> Technically 64-bit addressing for maintenance_work_mem is possible,
> but code base historically uses variables and constants of type "long",
> when process maintenance_work_mem value.

I agree. Ideally, we shouldn't use longs for anything ever. We should
likely adopt trying to remove the usages of them when possible.

I'd like to suggest you go about this patch slightly differently with
the end goal of removing the limitation from maintenance_work_mem,
work_mem, autovacuum_work_mem and logical_decoding_work_mem.

Patch 0001: Add a macro named something like WORK_MEM_KB_TO_BYTES()
and adjust all places where we do <work_mem_var> * 1024L to use this
new macro. Make the macro do the * 1024L as is done today so that this
patch is a simple refactor.
Patch 0002: Convert all places that use long and use Size instead.
Adjust WORK_MEM_KB_TO_BYTES to use a Size type rather than 1024L.

It might be wise to break 0002 down into individual GUCs as the patch
might become large.

I suspect we might have quite a large number of subtle bugs in our
code today due to using longs. 7340d9362 is an example of one that was
fixed recently.

David

  3. Re: Increase of maintenance_work_mem limit in 64-bit Windows

    Vladlen Popolitov <v.popolitov@postgrespro.ru> — 2024-09-23T09:01:31Z

    David Rowley писал(а) 2024-09-23 04:28:
> On Fri, 20 Sept 2024 at 01:55, Пополитов Владлен
> <v.popolitov@postgrespro.ru> wrote:
>> Currently PostgreSQL built on 64-bit Windows has 2Gb limit for
>> GUC variables due to sizeof(long)==4 used by Windows compilers.
>> Technically 64-bit addressing for maintenance_work_mem is possible,
>> but code base historically uses variables and constants of type 
>> "long",
>> when process maintenance_work_mem value.
> 
> I agree. Ideally, we shouldn't use longs for anything ever. We should
> likely adopt trying to remove the usages of them when possible.
> 
> I'd like to suggest you go about this patch slightly differently with
> the end goal of removing the limitation from maintenance_work_mem,
> work_mem, autovacuum_work_mem and logical_decoding_work_mem.
> 
> Patch 0001: Add a macro named something like WORK_MEM_KB_TO_BYTES()
> and adjust all places where we do <work_mem_var> * 1024L to use this
> new macro. Make the macro do the * 1024L as is done today so that this
> patch is a simple refactor.
> Patch 0002: Convert all places that use long and use Size instead.
> Adjust WORK_MEM_KB_TO_BYTES to use a Size type rather than 1024L.
> 
> It might be wise to break 0002 down into individual GUCs as the patch
> might become large.
> 
> I suspect we might have quite a large number of subtle bugs in our
> code today due to using longs. 7340d9362 is an example of one that was
> fixed recently.
> 
> David

Hi David,
Thank you for proposal, I looked at the patch and source code from this
point of view. In this approach we need to change all <work_mem_var>.
I counted the appearences of these vars in the code:
maintenance_work_mem appears 63 times in 20 files
work_mem appears 113 times in 48 files
logical_decoding_work_mem appears 10 times in 2 files
max_stack_depth appears 11 times in 3 files
wal_keep_size_mb appears 5 times in 3 files
min_wal_size_mb appears 5 times in 2 files
max_wal_size_mb appears 10 times in 2 files
wal_skip_threshold appears 5 times in 2 files
max_slot_wal_keep_size_mb appears 6 times in 3 files
wal_sender_timeout appears 23 times in 3 files
autovacuum_work_mem appears 11 times in 4 files
gin_pending_list_limit appears 8 times in 5 files
pendingListCleanupSize appears 2 times in 2 files
GinGetPendingListCleanupSize appears 2 times in 2 files

maintenance_work_mem appears 63 times and had only 4 cases, where "long"
is used (I fix it in patch). I also found, that this patch also fixed
autovacuum_work_mem , that has only 1 case - the same place in code as
maintenance_work_mem.

Now <work_mem_vars> in the code are processed based on the context: they 
are
assigned to Size, uint64, int64, double, long, int variables (last 2 
cases
need to fix) or multiplied by (uint64)1024, (Size)1024, 1024L (last case
needs to fix). Also signed value is used for max_stack_depth (-1 used as
error value). I am not sure, that we can solve all this cases by one
macro WORK_MEM_KB_TO_BYTES(). The code needs case by case check.

If I check the rest of the variables, the patch does not need
MAX_SIZE_T_KILOBYTES constant (I introduced it for variables, that are
already checked and fixed), it will contain only fixes in the types of
the variables and the constants.
It requires a lot of time to check all appearances and neighbour
code, but final patch will not be large, I do not expect a lot of
"long" in the rest of the code (only 4 case out of 63 needed to fix
for maintenance_work_mem).
What do you think about this approach?

-- 
Best regards,

Vladlen Popolitov.

  4. Re: Increase of maintenance_work_mem limit in 64-bit Windows

    David Rowley <dgrowleyml@gmail.com> — 2024-09-23T12:35:45Z

    On Mon, 23 Sept 2024 at 21:01, Vladlen Popolitov
<v.popolitov@postgrespro.ru> wrote:
> Thank you for proposal, I looked at the patch and source code from this
> point of view. In this approach we need to change all <work_mem_var>.
> I counted the appearences of these vars in the code:
> maintenance_work_mem appears 63 times in 20 files
> work_mem appears 113 times in 48 files
> logical_decoding_work_mem appears 10 times in 2 files
> max_stack_depth appears 11 times in 3 files
> wal_keep_size_mb appears 5 times in 3 files
> min_wal_size_mb appears 5 times in 2 files
> max_wal_size_mb appears 10 times in 2 files
> wal_skip_threshold appears 5 times in 2 files
> max_slot_wal_keep_size_mb appears 6 times in 3 files
> wal_sender_timeout appears 23 times in 3 files
> autovacuum_work_mem appears 11 times in 4 files
> gin_pending_list_limit appears 8 times in 5 files
> pendingListCleanupSize appears 2 times in 2 files
> GinGetPendingListCleanupSize appears 2 times in 2 files

Why do you think all of these appearances matter? I imagined all you
care about are when the values are multiplied by 1024.

> If I check the rest of the variables, the patch does not need
> MAX_SIZE_T_KILOBYTES constant (I introduced it for variables, that are
> already checked and fixed), it will contain only fixes in the types of
> the variables and the constants.
> It requires a lot of time to check all appearances and neighbour
> code, but final patch will not be large, I do not expect a lot of
> "long" in the rest of the code (only 4 case out of 63 needed to fix
> for maintenance_work_mem).
> What do you think about this approach?

I don't think you can do maintenance_work_mem without fixing work_mem
too. I don't think the hacks you've put into RI_Initial_Check() to
ensure you don't try to set work_mem beyond its allowed range are very
good. It effectively means that maintenance_work_mem does not do what
it's meant to for the initial validation of referential integrity
checks. If you're not planning on fixing work_mem too, would you just
propose to leave those hacks in there forever?

David

  5. Re: Increase of maintenance_work_mem limit in 64-bit Windows

    Vladlen Popolitov <v.popolitov@postgrespro.ru> — 2024-09-23T14:47:52Z

    David Rowley писал(а) 2024-09-23 15:35:
> On Mon, 23 Sept 2024 at 21:01, Vladlen Popolitov
> <v.popolitov@postgrespro.ru> wrote:
>> Thank you for proposal, I looked at the patch and source code from 
>> this
>> point of view. In this approach we need to change all <work_mem_var>.
>> I counted the appearences of these vars in the code:
>> maintenance_work_mem appears 63 times in 20 files
>> work_mem appears 113 times in 48 files
>> logical_decoding_work_mem appears 10 times in 2 files
>> max_stack_depth appears 11 times in 3 files
>> wal_keep_size_mb appears 5 times in 3 files
>> min_wal_size_mb appears 5 times in 2 files
>> max_wal_size_mb appears 10 times in 2 files
>> wal_skip_threshold appears 5 times in 2 files
>> max_slot_wal_keep_size_mb appears 6 times in 3 files
>> wal_sender_timeout appears 23 times in 3 files
>> autovacuum_work_mem appears 11 times in 4 files
>> gin_pending_list_limit appears 8 times in 5 files
>> pendingListCleanupSize appears 2 times in 2 files
>> GinGetPendingListCleanupSize appears 2 times in 2 files
> 
> Why do you think all of these appearances matter? I imagined all you
> care about are when the values are multiplied by 1024.
Common pattern in code - assign <work_mem_var> to local variable and 
send
local variable as parameter to function, then to nested function, and
somewhere deep multiply function parameter by 1024. It is why I needed 
to
check all appearances, most of them are correct.
>> If I check the rest of the variables, the patch does not need
>> MAX_SIZE_T_KILOBYTES constant (I introduced it for variables, that are
>> already checked and fixed), it will contain only fixes in the types of
>> the variables and the constants.
>> It requires a lot of time to check all appearances and neighbour
>> code, but final patch will not be large, I do not expect a lot of
>> "long" in the rest of the code (only 4 case out of 63 needed to fix
>> for maintenance_work_mem).
>> What do you think about this approach?
> 
> I don't think you can do maintenance_work_mem without fixing work_mem
> too. I don't think the hacks you've put into RI_Initial_Check() to
> ensure you don't try to set work_mem beyond its allowed range are very
> good. It effectively means that maintenance_work_mem does not do what
> it's meant to for the initial validation of referential integrity
> checks. If you're not planning on fixing work_mem too, would you just
> propose to leave those hacks in there forever?
I agree, it is better to fix all them together. I also do not like this
hack, it will be removed from the patch, if I check and change
all <work_mem_vars> at once.
I think, it will take about 1 week to fix and test all changes. I will
estimate the total volume of the changes and think, how to group them
in the patch ( I hope, it will be only one patch)

-- 
Best regards,

Vladlen Popolitov.

  6. Re: Increase of maintenance_work_mem limit in 64-bit Windows

    David Rowley <dgrowleyml@gmail.com> — 2024-09-23T22:07:23Z

    On Tue, 24 Sept 2024 at 02:47, Vladlen Popolitov
<v.popolitov@postgrespro.ru> wrote:
> I agree, it is better to fix all them together. I also do not like this
> hack, it will be removed from the patch, if I check and change
> all <work_mem_vars> at once.
> I think, it will take about 1 week to fix and test all changes. I will
> estimate the total volume of the changes and think, how to group them
> in the patch ( I hope, it will be only one patch)

There's a few places that do this:

Size maxBlockSize = ALLOCSET_DEFAULT_MAXSIZE;

/* choose the maxBlockSize to be no larger than 1/16 of work_mem */
while (16 * maxBlockSize > work_mem * 1024L)

I think since maxBlockSize is a Size variable, that the above should
probably be:

while (16 * maxBlockSize > (Size) work_mem * 1024)

Maybe there can be a precursor patch to fix all those to get rid of
the 'L' and cast to the type we're comparing to or assigning to rather
than trying to keep the result of the multiplication as a long.

David

  7. Re: Increase of maintenance_work_mem limit in 64-bit Windows

    Vladlen Popolitov <v.popolitov@postgrespro.ru> — 2024-09-24T07:19:13Z

    David Rowley писал(а) 2024-09-24 01:07:
> On Tue, 24 Sept 2024 at 02:47, Vladlen Popolitov
> <v.popolitov@postgrespro.ru> wrote:
>> I agree, it is better to fix all them together. I also do not like 
>> this
>> hack, it will be removed from the patch, if I check and change
>> all <work_mem_vars> at once.
>> I think, it will take about 1 week to fix and test all changes. I will
>> estimate the total volume of the changes and think, how to group them
>> in the patch ( I hope, it will be only one patch)
> 
> There's a few places that do this:
> 
> Size maxBlockSize = ALLOCSET_DEFAULT_MAXSIZE;
> 
> /* choose the maxBlockSize to be no larger than 1/16 of work_mem */
> while (16 * maxBlockSize > work_mem * 1024L)
> 
> I think since maxBlockSize is a Size variable, that the above should
> probably be:
> 
> while (16 * maxBlockSize > (Size) work_mem * 1024)
> 
> Maybe there can be a precursor patch to fix all those to get rid of
> the 'L' and cast to the type we're comparing to or assigning to rather
> than trying to keep the result of the multiplication as a long.
Yes. It is what I mean, when I wrote about the context - in this case
variable is used in "Size" context and the cast to Size type should be
used. It is why I need to check all places in code. I am going to do it
during this week.

-- 
Best regards,

Vladlen Popolitov.

  8. Re: Increase of maintenance_work_mem limit in 64-bit Windows

    Vladlen Popolitov <v.popolitov@postgrespro.ru> — 2024-09-30T21:30:40Z

    David Rowley писал(а) 2024-09-24 01:07:
> On Tue, 24 Sept 2024 at 02:47, Vladlen Popolitov
> <v.popolitov@postgrespro.ru> wrote:
>> I agree, it is better to fix all them together. I also do not like 
>> this
>> hack, it will be removed from the patch, if I check and change
>> all <work_mem_vars> at once.
>> I think, it will take about 1 week to fix and test all changes. I will
>> estimate the total volume of the changes and think, how to group them
>> in the patch ( I hope, it will be only one patch)
> 
> There's a few places that do this:
> 
> Size maxBlockSize = ALLOCSET_DEFAULT_MAXSIZE;
> 
> /* choose the maxBlockSize to be no larger than 1/16 of work_mem */
> while (16 * maxBlockSize > work_mem * 1024L)
> 
> I think since maxBlockSize is a Size variable, that the above should
> probably be:
> 
> while (16 * maxBlockSize > (Size) work_mem * 1024)
> 
> Maybe there can be a precursor patch to fix all those to get rid of
> the 'L' and cast to the type we're comparing to or assigning to rather
> than trying to keep the result of the multiplication as a long.

Hi

I rechecked all <work_mem_vars>, that depend on MAX_KILOBYTES limit and 
fixed
all casts that are affected by 4-bytes long type in Windows 64-bit. Now
next variables are limited by 2TB in all 64-bit systems:
maintenance_work_mem
work_mem
logical_decoding_work_mem
max_stack_depth
autovacuum_work_mem
gin_pending_list_limit
wal_skip_threshold
Also wal_keep_size_mb, min_wal_size_mb, max_wal_size_mb,
max_slot_wal_keep_size_mb are not affected by "long" cast.

  9. Re: Increase of maintenance_work_mem limit in 64-bit Windows

    Vladlen Popolitov <v.popolitov@postgrespro.ru> — 2024-10-30T19:24:16Z

    v.popolitov@postgrespro.ru писал(а) 2024-10-01 00:30:
> David Rowley писал(а) 2024-09-24 01:07:
>> On Tue, 24 Sept 2024 at 02:47, Vladlen Popolitov
>> <v.popolitov@postgrespro.ru> wrote:
>>> I agree, it is better to fix all them together. I also do not like 
>>> this
>>> hack, it will be removed from the patch, if I check and change
>>> all <work_mem_vars> at once.
>>> I think, it will take about 1 week to fix and test all changes. I 
>>> will
>>> estimate the total volume of the changes and think, how to group them
>>> in the patch ( I hope, it will be only one patch)
>> 
>> There's a few places that do this:
>> 
>> Size maxBlockSize = ALLOCSET_DEFAULT_MAXSIZE;
>> 
>> /* choose the maxBlockSize to be no larger than 1/16 of work_mem */
>> while (16 * maxBlockSize > work_mem * 1024L)
>> 
>> I think since maxBlockSize is a Size variable, that the above should
>> probably be:
>> 
>> while (16 * maxBlockSize > (Size) work_mem * 1024)
>> 
>> Maybe there can be a precursor patch to fix all those to get rid of
>> the 'L' and cast to the type we're comparing to or assigning to rather
>> than trying to keep the result of the multiplication as a long.
> 
> Hi
> 
> I rechecked all <work_mem_vars>, that depend on MAX_KILOBYTES limit and 
> fixed
> all casts that are affected by 4-bytes long type in Windows 64-bit. Now
> next variables are limited by 2TB in all 64-bit systems:
> maintenance_work_mem
> work_mem
> logical_decoding_work_mem
> max_stack_depth
> autovacuum_work_mem
> gin_pending_list_limit
> wal_skip_threshold
> Also wal_keep_size_mb, min_wal_size_mb, max_wal_size_mb,
> max_slot_wal_keep_size_mb are not affected by "long" cast.

Hi everyone.

The patch added to Commitfest:
https://commitfest.postgresql.org/50/5343/
-- 
Best regards,

Vladlen Popolitov.

  10. Re: Increase of maintenance_work_mem limit in 64-bit Windows

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-01-23T23:13:26Z

    v.popolitov@postgrespro.ru writes:
> [ v2-0001-work_mem_vars-limit-increased-in-64bit-Windows.patch ]

I took a brief look at this.  I think it's generally going in the
right direction, but you seem to be all over the place on how
you are doing the casts:

+	if (metadata->nPendingPages * GIN_PAGE_FREESIZE > cleanupSize * (Size)1024L)

+	scanEntry->matchBitmap = tbm_create(work_mem * INT64CONST(1024), NULL);

+	dead_items_info->max_bytes = vac_work_mem * (size_t)1024;

Is there a reason not to do them all the same way?  I'd suggest
"(Size) 1024", losing the "L" which has no purpose any more.
(And note project style is with a space after the cast.)
I don't like use of INT64CONST here because that forces an
unnecessarily expensive calculation in 32-bit machines.

I wonder if it'd be worth creating a macro rather than repeating
"* (Size) 1024" everywhere.  KILOBYTES_TO_BYTES(work_mem) seems
too wordy, but maybe we can think of a shorter name.

-	long		sort_threshold;
+	uint64		sort_threshold;

(1) Are you sure the related code doesn't need this to be signed?
(2) Even if it can be unsigned, why not Size or size_t?

-tbm_create(long maxbytes, dsa_area *dsa)
+tbm_create(double maxbytes, dsa_area *dsa)

Why "double"??

Also, do we need to widen the result of tbm_calculate_entries?
I see the clamp to INT_MAX-1, but should we try to get rid of
that?  (Or if not, should we reduce its result to "int"?)

-	long		sort_mem_bytes = sort_mem * 1024L;
+	int64		sort_mem_bytes = sort_mem * INT64CONST(1024);

Wrong type surely, and even more so here:

-	long		work_mem_bytes = work_mem * 1024L;
+	double		work_mem_bytes = work_mem * INT64CONST(1024);

If there's actually a reason for this scattershot approach to
new data types, you need to explain what the plan is.  I'd
have expected a push to replace "long" with "Size", or maybe
use size_t (or ssize_t when we need a signed type).
 
 /* upper limit for GUC variables measured in kilobytes of memory */
 /* note that various places assume the byte size fits in a "long" variable */
-#if SIZEOF_SIZE_T > 4 && SIZEOF_LONG > 4
+#if SIZEOF_SIZE_T > 4
 #define MAX_KILOBYTES	INT_MAX
 #else
 #define MAX_KILOBYTES	(INT_MAX / 1024)

This is pretty much the crux of the whole thing, and you didn't
fix/remove the comment you falsified.

			regards, tom lane

  11. Re: Increase of maintenance_work_mem limit in 64-bit Windows

    Vladlen Popolitov <v.popolitov@postgrespro.ru> — 2025-01-28T17:16:18Z

    Tom Lane писал(а) 2025-01-24 06:13:
> v.popolitov@postgrespro.ru writes:
>> [ v2-0001-work_mem_vars-limit-increased-in-64bit-Windows.patch ]
Hi Tom,

Thank you for the interest to the patch and comments.

Comment A.
> Is there a reason not to do them all the same way?  I'd suggest
> "(Size) 1024", losing the "L" which has no purpose any more.
> (And note project style is with a space after the cast.)
> I don't like use of INT64CONST here because that forces an
> unnecessarily expensive calculation in 32-bit machines.

Trying to fix 64-bit code I did not consider 32-bit code seriously,
but your comment changed my mind. I tried to cast everything to
lvalue type (it was the reason of many cast kinds). Now I changed
almost everything to (Size)1024, what is 32 and 64 bit friendly.

Only one exceptions in src/backend/commands/vacuumparallel.c:378
shared->dead_items_info.max_bytes = vac_work_mem * (size_t)1024;
max_bytes declared as size_t, I did cast to the same type (I cannot
guarranty, that size_t and Size are equivalent in all systems)

Comment B.
> I wonder if it'd be worth creating a macro rather than repeating
> "* (Size) 1024" everywhere.  KILOBYTES_TO_BYTES(work_mem) seems
> too wordy, but maybe we can think of a shorter name.
I looked in source for all variables overflow connected to "work_mem"
variables and fixed them. I did not want to include additional code
and I tried to keep the patch as small as possible. Source has other
locations with KB to bytes conversions, but it was not the goal of
this patch.

Comment C.
> -	long		sort_threshold;
> +	uint64		sort_threshold;
> 
> (1) Are you sure the related code doesn't need this to be signed?
> (2) Even if it can be unsigned, why not Size or size_t?
Changed to Size from uint64. It should not be signed, it is compared
with positive values later and finaly casted to uint32.

Comment D.
> -	long		sort_mem_bytes = sort_mem * 1024L;
> +	int64		sort_mem_bytes = sort_mem * INT64CONST(1024);
> 
> Wrong type surely, and even more so here:
> 
> -	long		work_mem_bytes = work_mem * 1024L;
> +	double		work_mem_bytes = work_mem * INT64CONST(1024);
> 
> If there's actually a reason for this scattershot approach to
> new data types, you need to explain what the plan is.  I'd
> have expected a push to replace "long" with "Size", or maybe
> use size_t (or ssize_t when we need a signed type).
INT64CONST(1024) changed to (Size)1024, but I keep new types
"int64" and "double" here.
1) sort_mem_bytes is used as int64 variable later, passed to
function with int64 parameter. "Size" type is not used in
this case.
2) work_mem_bytes is compared only with double values later, it
does not need additional casts in this case. "Size" type is not
used in this case.

Comment E.
> -tbm_create(long maxbytes, dsa_area *dsa)
> +tbm_create(double maxbytes, dsa_area *dsa)
> 
> Why "double"??
> 
> Also, do we need to widen the result of tbm_calculate_entries?
> I see the clamp to INT_MAX-1, but should we try to get rid of
> that?  (Or if not, should we reduce its result to "int"?)
Agree. It was "double" due to usage of this variable as parameter
in tbm_calculate_entries(double maxbytes), but really
tbm_calculate_entries() does not need this type, it again
converted to "long" local variable. I changed parameter,
local variable and return value of tbm_calculate_entries()
to Size.

New version of diff:
-tbm_create(long maxbytes, dsa_area *dsa)
+tbm_create(Size maxbytes, dsa_area *dsa)

-long
-tbm_calculate_entries(double maxbytes)
+Size
+tbm_calculate_entries(Size maxbytes)
  {
-       long            nbuckets;
+       Size            nbuckets;

Also tbm_calculate_entries() is used in assignment to "long"
variable maxentries. This variable is the part of other code,
I did not touch it, only added explicit cast to long:

-       maxentries = tbm_calculate_entries(work_mem * 1024L);
+       maxentries = (long)tbm_calculate_entries(work_mem * (Size)1024);


Summary:
I did fixes in patch for A,B,C,E comments.
Comment D - I gave explanation, why I changed long to int64 and double
instead of Size, I hope it is logical and you will be satisfied.

-- 
Best regards,

Vladlen Popolitov.

  12. Re: Increase of maintenance_work_mem limit in 64-bit Windows

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-01-31T19:00:49Z

    Vladlen Popolitov <v.popolitov@postgrespro.ru> writes:
> Only one exceptions in src/backend/commands/vacuumparallel.c:378
> shared->dead_items_info.max_bytes = vac_work_mem * (size_t)1024;
> max_bytes declared as size_t, I did cast to the same type (I cannot
> guarranty, that size_t and Size are equivalent in all systems)

I direct your attention to c.h:

/*
 * Size
 *		Size of any memory resident object, as returned by sizeof.
 */
typedef size_t Size;

It used to be possible for them to be different types, but not
anymore.  But I take your point that using "size_t" instead
makes sense if that's how the target variable is declared.
(And I feel no need to replace size_t with Size or vice versa
in code that's already correct.)

>> I wonder if it'd be worth creating a macro rather than repeating
>> "* (Size) 1024" everywhere.  KILOBYTES_TO_BYTES(work_mem) seems
>> too wordy, but maybe we can think of a shorter name.

I agree that this wasn't such a great idea, mainly because we did
not end up using exactly "Size" everywhere.

> 1) sort_mem_bytes is used as int64 variable later, passed to
> function with int64 parameter. "Size" type is not used in
> this case.
> 2) work_mem_bytes is compared only with double values later, it
> does not need additional casts in this case. "Size" type is not
> used in this case.

Agreed on these.

> Agree. It was "double" due to usage of this variable as parameter
> in tbm_calculate_entries(double maxbytes), but really
> tbm_calculate_entries() does not need this type, it again
> converted to "long" local variable. I changed parameter,
> local variable and return value of tbm_calculate_entries()
> to Size.

Agreed on the parameter being Size, but I made the return type
be int to emphasize that we're restricting the result to
integer range.  (It looks like additional work would be needed
to let it exceed INT_MAX; if anyone ever feels like doing that
work, they can change the result type again.)

> Also tbm_calculate_entries() is used in assignment to "long"
> variable maxentries.

I concluded there that we should make maxentries "double",
on the same reasoning as you have above: it's only used in
calculations with other double variables.

Pushed with some minor additional cleanups.  Many thanks for
working on this!  It was a TODO for ages.

			regards, tom lane