Increase of maintenance_work_mem limit in 64-bit Windows
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
Attachments:
v1-0001-maintenance_work_mem-limit-increased-in-64-bit-Wi.patchapplication/octet-streamDownload
From f9ad96e50abb17115a6c3292ec0cfb750db33af3 Mon Sep 17 00:00:00 2001
From: Vladlen Popolitov <v.popolitov@postgrespro.ru>
Date: Thu, 19 Sep 2024 14:17:58 +0300
Subject: [PATCH v1] maintenance_work_mem limit increased in 64-bit Windows
---
src/backend/access/hash/hash.c | 2 +-
src/backend/access/heap/vacuumlazy.c | 2 +-
src/backend/commands/vacuumparallel.c | 2 +-
src/backend/optimizer/path/costsize.c | 2 +-
src/backend/utils/adt/ri_triggers.c | 23 +++++++++++++++++++----
src/backend/utils/misc/guc_tables.c | 10 +++++++++-
6 files changed, 32 insertions(+), 9 deletions(-)
diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index 5ce3609394..0063902021 100644
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -120,7 +120,7 @@ hashbuild(Relation heap, Relation index, IndexInfo *indexInfo)
double reltuples;
double allvisfrac;
uint32 num_buckets;
- long sort_threshold;
+ uint64 sort_threshold;
HashBuildState buildstate;
/*
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index d82aa3d489..eb658f66f1 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -2883,7 +2883,7 @@ dead_items_alloc(LVRelState *vacrel, int nworkers)
*/
dead_items_info = (VacDeadItemsInfo *) palloc(sizeof(VacDeadItemsInfo));
- dead_items_info->max_bytes = vac_work_mem * 1024L;
+ dead_items_info->max_bytes = vac_work_mem * (size_t)1024;
dead_items_info->num_items = 0;
vacrel->dead_items_info = dead_items_info;
diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.c
index 22c057fe61..208c99c48c 100644
--- a/src/backend/commands/vacuumparallel.c
+++ b/src/backend/commands/vacuumparallel.c
@@ -373,7 +373,7 @@ parallel_vacuum_init(Relation rel, Relation *indrels, int nindexes,
(nindexes_mwm > 0) ?
maintenance_work_mem / Min(parallel_workers, nindexes_mwm) :
maintenance_work_mem;
- shared->dead_items_info.max_bytes = vac_work_mem * 1024L;
+ shared->dead_items_info.max_bytes = vac_work_mem * (size_t)1024;
/* Prepare DSA space for dead items */
dead_items = TidStoreCreateShared(shared->dead_items_info.max_bytes,
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index e1523d15df..795a26e42a 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -1903,7 +1903,7 @@ cost_tuplesort(Cost *startup_cost, Cost *run_cost,
double input_bytes = relation_byte_size(tuples, width);
double output_bytes;
double output_tuples;
- long sort_mem_bytes = sort_mem * 1024L;
+ int64 sort_mem_bytes = sort_mem * INT64CONST(1024);
/*
* We want to be sure the cost of a sort is never estimated as zero, even
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 6896e1ae63..eb3423f5ca 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -1617,11 +1617,26 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
* care of undoing the setting on error.
*/
save_nestlevel = NewGUCNestLevel();
+ {
+ int work_mem_new_value;
+#if SIZEOF_SIZE_T > 4 && SIZEOF_LONG > 4
+ work_mem_new_value = maintenance_work_mem;
+#else
+ if (maintenance_work_mem >= (INT_MAX / 1024))
+ {
+ work_mem_new_value = (INT_MAX / 1024);
+ }
+ else
+ {
+ work_mem_new_value = maintenance_work_mem;
+ }
+#endif
- snprintf(workmembuf, sizeof(workmembuf), "%d", maintenance_work_mem);
- (void) set_config_option("work_mem", workmembuf,
- PGC_USERSET, PGC_S_SESSION,
- GUC_ACTION_SAVE, true, 0, false);
+ snprintf(workmembuf, sizeof(workmembuf), "%d", work_mem_new_value);
+ (void) set_config_option("work_mem", workmembuf,
+ PGC_USERSET, PGC_S_SESSION,
+ GUC_ACTION_SAVE, true, 0, false);
+ }
(void) set_config_option("hash_mem_multiplier", "1",
PGC_USERSET, PGC_S_SESSION,
GUC_ACTION_SAVE, true, 0, false);
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 686309db58..8cbadaa02a 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -722,6 +722,14 @@ const char *const config_group_names[] =
StaticAssertDecl(lengthof(config_group_names) == (DEVELOPER_OPTIONS + 1),
"array length mismatch");
+/* upper limit for GUC variables measured in kilobytes of memory */
+/* only for GUC variables that are not affected by a "long" variable */
+#if SIZEOF_SIZE_T > 4
+#define MAX_SIZE_T_KILOBYTES INT_MAX
+#else
+#define MAX_SIZE_T_KILOBYTES (INT_MAX / 1024)
+#endif
+
/*
* Displayable names for GUC variable types (enum config_type)
*
@@ -2520,7 +2528,7 @@ struct config_int ConfigureNamesInt[] =
GUC_UNIT_KB
},
&maintenance_work_mem,
- 65536, 64, MAX_KILOBYTES,
+ 65536, 64, MAX_SIZE_T_KILOBYTES,
NULL, NULL, NULL
},
--
2.42.0.windows.2
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
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.
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
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 filesWhy 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.
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
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.
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.
Attachments:
v2-0001-work_mem_vars-limit-increased-in-64bit-Windows.patchtext/x-diff; name=v2-0001-work_mem_vars-limit-increased-in-64bit-Windows.patchDownload
From 6d275cb66cb39b1f209a6b43db2ce377ec0d7ba8 Mon Sep 17 00:00:00 2001
From: Vladlen Popolitov <v.popolitov@postgrespro.ru>
Date: Tue, 1 Oct 2024 00:10:37 +0300
Subject: [PATCH v2] work_mem_vars limit increased in 64bit Windows
---
src/backend/access/gin/ginfast.c | 2 +-
src/backend/access/gin/ginget.c | 2 +-
src/backend/access/hash/hash.c | 2 +-
src/backend/access/heap/vacuumlazy.c | 2 +-
src/backend/access/nbtree/nbtpage.c | 2 +-
src/backend/commands/vacuumparallel.c | 2 +-
src/backend/executor/execUtils.c | 2 +-
src/backend/executor/nodeBitmapIndexscan.c | 2 +-
src/backend/executor/nodeBitmapOr.c | 2 +-
src/backend/nodes/tidbitmap.c | 6 +++---
src/backend/optimizer/path/costsize.c | 12 ++++++------
src/backend/replication/logical/reorderbuffer.c | 6 +++---
src/backend/tcop/postgres.c | 4 ++--
src/backend/utils/sort/tuplestore.c | 2 +-
src/include/nodes/tidbitmap.h | 2 +-
src/include/utils/guc.h | 2 +-
16 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/src/backend/access/gin/ginfast.c b/src/backend/access/gin/ginfast.c
index eeca3ed318..d707e459bb 100644
--- a/src/backend/access/gin/ginfast.c
+++ b/src/backend/access/gin/ginfast.c
@@ -456,7 +456,7 @@ ginHeapTupleFastInsert(GinState *ginstate, GinTupleCollector *collector)
* ginInsertCleanup() should not be called inside our CRIT_SECTION.
*/
cleanupSize = GinGetPendingListCleanupSize(index);
- if (metadata->nPendingPages * GIN_PAGE_FREESIZE > cleanupSize * 1024L)
+ if (metadata->nPendingPages * GIN_PAGE_FREESIZE > cleanupSize * (Size)1024L)
needCleanup = true;
UnlockReleaseBuffer(metabuffer);
diff --git a/src/backend/access/gin/ginget.c b/src/backend/access/gin/ginget.c
index 0b4f2ebadb..1f295d5907 100644
--- a/src/backend/access/gin/ginget.c
+++ b/src/backend/access/gin/ginget.c
@@ -125,7 +125,7 @@ collectMatchBitmap(GinBtreeData *btree, GinBtreeStack *stack,
Form_pg_attribute attr;
/* Initialize empty bitmap result */
- scanEntry->matchBitmap = tbm_create(work_mem * 1024L, NULL);
+ scanEntry->matchBitmap = tbm_create(work_mem * INT64CONST(1024), NULL);
/* Null query cannot partial-match anything */
if (scanEntry->isPartialMatch &&
diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index 5ce3609394..0063902021 100644
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -120,7 +120,7 @@ hashbuild(Relation heap, Relation index, IndexInfo *indexInfo)
double reltuples;
double allvisfrac;
uint32 num_buckets;
- long sort_threshold;
+ uint64 sort_threshold;
HashBuildState buildstate;
/*
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index d82aa3d489..eb658f66f1 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -2883,7 +2883,7 @@ dead_items_alloc(LVRelState *vacrel, int nworkers)
*/
dead_items_info = (VacDeadItemsInfo *) palloc(sizeof(VacDeadItemsInfo));
- dead_items_info->max_bytes = vac_work_mem * 1024L;
+ dead_items_info->max_bytes = vac_work_mem * (size_t)1024;
dead_items_info->num_items = 0;
vacrel->dead_items_info = dead_items_info;
diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c
index 01bbece6bf..4ab3f6129f 100644
--- a/src/backend/access/nbtree/nbtpage.c
+++ b/src/backend/access/nbtree/nbtpage.c
@@ -2969,7 +2969,7 @@ _bt_pendingfsm_init(Relation rel, BTVacState *vstate, bool cleanuponly)
* int overflow here.
*/
vstate->bufsize = 256;
- maxbufsize = (work_mem * 1024L) / sizeof(BTPendingFSM);
+ maxbufsize = (work_mem * INT64CONST(1024)) / sizeof(BTPendingFSM);
maxbufsize = Min(maxbufsize, INT_MAX);
maxbufsize = Min(maxbufsize, MaxAllocSize / sizeof(BTPendingFSM));
/* Stay sane with small work_mem */
diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.c
index 4fd6574e12..c4312e314c 100644
--- a/src/backend/commands/vacuumparallel.c
+++ b/src/backend/commands/vacuumparallel.c
@@ -375,7 +375,7 @@ parallel_vacuum_init(Relation rel, Relation *indrels, int nindexes,
(nindexes_mwm > 0) ?
maintenance_work_mem / Min(parallel_workers, nindexes_mwm) :
maintenance_work_mem;
- shared->dead_items_info.max_bytes = vac_work_mem * 1024L;
+ shared->dead_items_info.max_bytes = vac_work_mem * (size_t)1024;
/* Prepare DSA space for dead items */
dead_items = TidStoreCreateShared(shared->dead_items_info.max_bytes,
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index 5737f9f4eb..82e3662880 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -323,7 +323,7 @@ CreateWorkExprContext(EState *estate)
Size maxBlockSize = ALLOCSET_DEFAULT_MAXSIZE;
/* choose the maxBlockSize to be no larger than 1/16 of work_mem */
- while (16 * maxBlockSize > work_mem * 1024L)
+ while (16 * maxBlockSize > work_mem * (Size)1024L)
maxBlockSize >>= 1;
if (maxBlockSize < ALLOCSET_DEFAULT_INITSIZE)
diff --git a/src/backend/executor/nodeBitmapIndexscan.c b/src/backend/executor/nodeBitmapIndexscan.c
index 6df8e17ec8..f6aba014db 100644
--- a/src/backend/executor/nodeBitmapIndexscan.c
+++ b/src/backend/executor/nodeBitmapIndexscan.c
@@ -91,7 +91,7 @@ MultiExecBitmapIndexScan(BitmapIndexScanState *node)
else
{
/* XXX should we use less than work_mem for this? */
- tbm = tbm_create(work_mem * 1024L,
+ tbm = tbm_create(work_mem * INT64CONST(1024),
((BitmapIndexScan *) node->ss.ps.plan)->isshared ?
node->ss.ps.state->es_query_dsa : NULL);
}
diff --git a/src/backend/executor/nodeBitmapOr.c b/src/backend/executor/nodeBitmapOr.c
index 7029536c64..e189253d30 100644
--- a/src/backend/executor/nodeBitmapOr.c
+++ b/src/backend/executor/nodeBitmapOr.c
@@ -143,7 +143,7 @@ MultiExecBitmapOr(BitmapOrState *node)
if (result == NULL) /* first subplan */
{
/* XXX should we use less than work_mem for this? */
- result = tbm_create(work_mem * 1024L,
+ result = tbm_create(work_mem * INT64CONST(1024),
((BitmapOr *) node->ps.plan)->isshared ?
node->ps.state->es_query_dsa : NULL);
}
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index e8ab5d78fc..c438188e2e 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -263,7 +263,7 @@ static int tbm_shared_comparator(const void *left, const void *right,
* be allocated from the DSA.
*/
TIDBitmap *
-tbm_create(long maxbytes, dsa_area *dsa)
+tbm_create(double maxbytes, dsa_area *dsa)
{
TIDBitmap *tbm;
@@ -1541,7 +1541,7 @@ pagetable_free(pagetable_hash *pagetable, void *pointer)
long
tbm_calculate_entries(double maxbytes)
{
- long nbuckets;
+ Size nbuckets;
/*
* Estimate number of hashtable entries we can have within maxbytes. This
@@ -1554,5 +1554,5 @@ tbm_calculate_entries(double maxbytes)
nbuckets = Min(nbuckets, INT_MAX - 1); /* safety limit */
nbuckets = Max(nbuckets, 16); /* sanity limit */
- return nbuckets;
+ return (long)nbuckets;
}
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index e1523d15df..069095ef73 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -1903,7 +1903,7 @@ cost_tuplesort(Cost *startup_cost, Cost *run_cost,
double input_bytes = relation_byte_size(tuples, width);
double output_bytes;
double output_tuples;
- long sort_mem_bytes = sort_mem * 1024L;
+ int64 sort_mem_bytes = sort_mem * INT64CONST(1024);
/*
* We want to be sure the cost of a sort is never estimated as zero, even
@@ -2488,7 +2488,7 @@ cost_material(Path *path,
Cost startup_cost = input_startup_cost;
Cost run_cost = input_total_cost - input_startup_cost;
double nbytes = relation_byte_size(tuples, width);
- long work_mem_bytes = work_mem * 1024L;
+ double work_mem_bytes = work_mem * INT64CONST(1024);
path->rows = tuples;
@@ -3983,7 +3983,7 @@ final_cost_mergejoin(PlannerInfo *root, MergePath *path,
else if (enable_material && innersortkeys != NIL &&
relation_byte_size(inner_path_rows,
inner_path->pathtarget->width) >
- (work_mem * 1024L))
+ (double)(work_mem * INT64CONST(1024)))
path->materialize_inner = true;
else
path->materialize_inner = false;
@@ -4618,7 +4618,7 @@ cost_rescan(PlannerInfo *root, Path *path,
Cost run_cost = cpu_tuple_cost * path->rows;
double nbytes = relation_byte_size(path->rows,
path->pathtarget->width);
- long work_mem_bytes = work_mem * 1024L;
+ double work_mem_bytes = work_mem * INT64CONST(1024);
if (nbytes > work_mem_bytes)
{
@@ -4645,7 +4645,7 @@ cost_rescan(PlannerInfo *root, Path *path,
Cost run_cost = cpu_operator_cost * path->rows;
double nbytes = relation_byte_size(path->rows,
path->pathtarget->width);
- long work_mem_bytes = work_mem * 1024L;
+ double work_mem_bytes = work_mem * INT64CONST(1024);
if (nbytes > work_mem_bytes)
{
@@ -6480,7 +6480,7 @@ compute_bitmap_pages(PlannerInfo *root, RelOptInfo *baserel,
* the bitmap at one time.)
*/
heap_pages = Min(pages_fetched, baserel->pages);
- maxentries = tbm_calculate_entries(work_mem * 1024L);
+ maxentries = tbm_calculate_entries(work_mem * INT64CONST(1024));
if (loop_count > 1)
{
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 22bcf171ff..fe87872eb5 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -3638,7 +3638,7 @@ ReorderBufferCheckMemoryLimit(ReorderBuffer *rb)
* haven't exceeded the memory limit.
*/
if (debug_logical_replication_streaming == DEBUG_LOGICAL_REP_STREAMING_BUFFERED &&
- rb->size < logical_decoding_work_mem * 1024L)
+ rb->size < logical_decoding_work_mem * (Size)1024L)
return;
/*
@@ -3651,7 +3651,7 @@ ReorderBufferCheckMemoryLimit(ReorderBuffer *rb)
* because a user can reduce the logical_decoding_work_mem to a smaller
* value before the most recent change.
*/
- while (rb->size >= logical_decoding_work_mem * 1024L ||
+ while (rb->size >= logical_decoding_work_mem * (Size)1024L ||
(debug_logical_replication_streaming == DEBUG_LOGICAL_REP_STREAMING_IMMEDIATE &&
rb->size > 0))
{
@@ -3694,7 +3694,7 @@ ReorderBufferCheckMemoryLimit(ReorderBuffer *rb)
}
/* We must be under the memory limit now. */
- Assert(rb->size < logical_decoding_work_mem * 1024L);
+ Assert(rb->size < logical_decoding_work_mem * (Size)1024L);
}
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 7f5eada9d4..cc9e838d54 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -126,7 +126,7 @@ typedef struct BindParamCbData
*/
/* max_stack_depth converted to bytes for speed of checking */
-static long max_stack_depth_bytes = 100 * 1024L;
+static int64 max_stack_depth_bytes = 100 * 1024L;
/*
* Stack base pointer -- initialized by PostmasterMain and inherited by
@@ -3627,7 +3627,7 @@ check_max_stack_depth(int *newval, void **extra, GucSource source)
void
assign_max_stack_depth(int newval, void *extra)
{
- long newval_bytes = newval * 1024L;
+ int64 newval_bytes = newval * INT64CONST(1024);
max_stack_depth_bytes = newval_bytes;
}
diff --git a/src/backend/utils/sort/tuplestore.c b/src/backend/utils/sort/tuplestore.c
index a720d70200..bbc6f3047f 100644
--- a/src/backend/utils/sort/tuplestore.c
+++ b/src/backend/utils/sort/tuplestore.c
@@ -265,7 +265,7 @@ tuplestore_begin_common(int eflags, bool interXact, int maxKBytes)
state->truncated = false;
state->usedDisk = false;
state->maxSpace = 0;
- state->allowedMem = maxKBytes * 1024L;
+ state->allowedMem = maxKBytes * INT64CONST(1024);
state->availMem = state->allowedMem;
state->myfile = NULL;
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index 1945f0639b..bfde9701f7 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -48,7 +48,7 @@ typedef struct TBMIterateResult
/* function prototypes in nodes/tidbitmap.c */
-extern TIDBitmap *tbm_create(long maxbytes, dsa_area *dsa);
+extern TIDBitmap *tbm_create(double maxbytes, dsa_area *dsa);
extern void tbm_free(TIDBitmap *tbm);
extern void tbm_free_shared_area(dsa_area *dsa, dsa_pointer dp);
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 840b0fe57f..7f716483aa 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -19,7 +19,7 @@
/* 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)
--
2.42.0.windows.2
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.
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
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.
Attachments:
v3-0001-work_mem_vars-limit-increased-in-64bit-Windows.patchtext/x-diff; name=v3-0001-work_mem_vars-limit-increased-in-64bit-Windows.patchDownload
From 015e60140e62439fe6a45afc2c2fcdf464514219 Mon Sep 17 00:00:00 2001
From: Vladlen Popolitov <v.popolitov@postgrespro.ru>
Date: Tue, 28 Jan 2025 18:34:17 +0700
Subject: [PATCH v3] work_mem_vars limit increased in 64bit Windows
---
src/backend/access/gin/ginfast.c | 2 +-
src/backend/access/gin/ginget.c | 2 +-
src/backend/access/hash/hash.c | 4 ++--
src/backend/access/heap/vacuumlazy.c | 2 +-
src/backend/access/nbtree/nbtpage.c | 2 +-
src/backend/commands/vacuumparallel.c | 2 +-
src/backend/executor/execUtils.c | 2 +-
src/backend/executor/nodeBitmapIndexscan.c | 2 +-
src/backend/executor/nodeBitmapOr.c | 2 +-
src/backend/nodes/tidbitmap.c | 8 ++++----
src/backend/optimizer/path/costsize.c | 12 ++++++------
src/backend/replication/logical/reorderbuffer.c | 6 +++---
src/backend/utils/misc/stack_depth.c | 4 ++--
src/backend/utils/sort/tuplestore.c | 2 +-
src/include/nodes/tidbitmap.h | 4 ++--
src/include/utils/guc.h | 3 +--
16 files changed, 29 insertions(+), 30 deletions(-)
diff --git a/src/backend/access/gin/ginfast.c b/src/backend/access/gin/ginfast.c
index 97b9684800..73cbbe4bf1 100644
--- a/src/backend/access/gin/ginfast.c
+++ b/src/backend/access/gin/ginfast.c
@@ -456,7 +456,7 @@ ginHeapTupleFastInsert(GinState *ginstate, GinTupleCollector *collector)
* ginInsertCleanup() should not be called inside our CRIT_SECTION.
*/
cleanupSize = GinGetPendingListCleanupSize(index);
- if (metadata->nPendingPages * GIN_PAGE_FREESIZE > cleanupSize * 1024L)
+ if (metadata->nPendingPages * GIN_PAGE_FREESIZE > cleanupSize * (Size)1024)
needCleanup = true;
UnlockReleaseBuffer(metabuffer);
diff --git a/src/backend/access/gin/ginget.c b/src/backend/access/gin/ginget.c
index 330805626e..287f83c763 100644
--- a/src/backend/access/gin/ginget.c
+++ b/src/backend/access/gin/ginget.c
@@ -125,7 +125,7 @@ collectMatchBitmap(GinBtreeData *btree, GinBtreeStack *stack,
CompactAttribute *attr;
/* Initialize empty bitmap result */
- scanEntry->matchBitmap = tbm_create(work_mem * 1024L, NULL);
+ scanEntry->matchBitmap = tbm_create(work_mem * (Size)1024, NULL);
/* Null query cannot partial-match anything */
if (scanEntry->isPartialMatch &&
diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index f950b9925f..9bde002334 100644
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -120,7 +120,7 @@ hashbuild(Relation heap, Relation index, IndexInfo *indexInfo)
double reltuples;
double allvisfrac;
uint32 num_buckets;
- long sort_threshold;
+ Size sort_threshold;
HashBuildState buildstate;
/*
@@ -155,7 +155,7 @@ hashbuild(Relation heap, Relation index, IndexInfo *indexInfo)
* one page. Also, "initial index size" accounting does not include the
* metapage, nor the first bitmap page.
*/
- sort_threshold = (maintenance_work_mem * 1024L) / BLCKSZ;
+ sort_threshold = (maintenance_work_mem * (Size)1024) / BLCKSZ;
if (index->rd_rel->relpersistence != RELPERSISTENCE_TEMP)
sort_threshold = Min(sort_threshold, NBuffers);
else
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 5ed43e4391..09f8f602d7 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -3037,7 +3037,7 @@ dead_items_alloc(LVRelState *vacrel, int nworkers)
*/
dead_items_info = (VacDeadItemsInfo *) palloc(sizeof(VacDeadItemsInfo));
- dead_items_info->max_bytes = vac_work_mem * 1024L;
+ dead_items_info->max_bytes = vac_work_mem * (Size)1024;
dead_items_info->num_items = 0;
vacrel->dead_items_info = dead_items_info;
diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c
index 111675d2ef..f648288bfe 100644
--- a/src/backend/access/nbtree/nbtpage.c
+++ b/src/backend/access/nbtree/nbtpage.c
@@ -2969,7 +2969,7 @@ _bt_pendingfsm_init(Relation rel, BTVacState *vstate, bool cleanuponly)
* int overflow here.
*/
vstate->bufsize = 256;
- maxbufsize = (work_mem * 1024L) / sizeof(BTPendingFSM);
+ maxbufsize = (work_mem * (Size)1024) / sizeof(BTPendingFSM);
maxbufsize = Min(maxbufsize, INT_MAX);
maxbufsize = Min(maxbufsize, MaxAllocSize / sizeof(BTPendingFSM));
/* Stay sane with small work_mem */
diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.c
index 0d92e694d6..a5361d36e7 100644
--- a/src/backend/commands/vacuumparallel.c
+++ b/src/backend/commands/vacuumparallel.c
@@ -375,7 +375,7 @@ parallel_vacuum_init(Relation rel, Relation *indrels, int nindexes,
(nindexes_mwm > 0) ?
maintenance_work_mem / Min(parallel_workers, nindexes_mwm) :
maintenance_work_mem;
- shared->dead_items_info.max_bytes = vac_work_mem * 1024L;
+ shared->dead_items_info.max_bytes = vac_work_mem * (size_t)1024;
/* Prepare DSA space for dead items */
dead_items = TidStoreCreateShared(shared->dead_items_info.max_bytes,
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index 7c539de5cf..1449d4ee2c 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -325,7 +325,7 @@ CreateWorkExprContext(EState *estate)
Size maxBlockSize = ALLOCSET_DEFAULT_MAXSIZE;
/* choose the maxBlockSize to be no larger than 1/16 of work_mem */
- while (16 * maxBlockSize > work_mem * 1024L)
+ while (16 * maxBlockSize > work_mem * (Size)1024)
maxBlockSize >>= 1;
if (maxBlockSize < ALLOCSET_DEFAULT_INITSIZE)
diff --git a/src/backend/executor/nodeBitmapIndexscan.c b/src/backend/executor/nodeBitmapIndexscan.c
index d3ef5a0004..11d8041459 100644
--- a/src/backend/executor/nodeBitmapIndexscan.c
+++ b/src/backend/executor/nodeBitmapIndexscan.c
@@ -91,7 +91,7 @@ MultiExecBitmapIndexScan(BitmapIndexScanState *node)
else
{
/* XXX should we use less than work_mem for this? */
- tbm = tbm_create(work_mem * 1024L,
+ tbm = tbm_create(work_mem * (Size)1024,
((BitmapIndexScan *) node->ss.ps.plan)->isshared ?
node->ss.ps.state->es_query_dsa : NULL);
}
diff --git a/src/backend/executor/nodeBitmapOr.c b/src/backend/executor/nodeBitmapOr.c
index a9ede1c108..aa1b843d1e 100644
--- a/src/backend/executor/nodeBitmapOr.c
+++ b/src/backend/executor/nodeBitmapOr.c
@@ -143,7 +143,7 @@ MultiExecBitmapOr(BitmapOrState *node)
if (result == NULL) /* first subplan */
{
/* XXX should we use less than work_mem for this? */
- result = tbm_create(work_mem * 1024L,
+ result = tbm_create(work_mem * (Size)1024,
((BitmapOr *) node->ps.plan)->isshared ?
node->ps.state->es_query_dsa : NULL);
}
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index af6ac64b3f..cf9952ca1a 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -263,7 +263,7 @@ static int tbm_shared_comparator(const void *left, const void *right,
* be allocated from the DSA.
*/
TIDBitmap *
-tbm_create(long maxbytes, dsa_area *dsa)
+tbm_create(Size maxbytes, dsa_area *dsa)
{
TIDBitmap *tbm;
@@ -1539,10 +1539,10 @@ pagetable_free(pagetable_hash *pagetable, void *pointer)
*
* Estimate number of hashtable entries we can have within maxbytes.
*/
-long
-tbm_calculate_entries(double maxbytes)
+Size
+tbm_calculate_entries(Size maxbytes)
{
- long nbuckets;
+ Size nbuckets;
/*
* Estimate number of hashtable entries we can have within maxbytes. This
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index ec004ed949..632e03966b 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -1903,7 +1903,7 @@ cost_tuplesort(Cost *startup_cost, Cost *run_cost,
double input_bytes = relation_byte_size(tuples, width);
double output_bytes;
double output_tuples;
- long sort_mem_bytes = sort_mem * 1024L;
+ int64 sort_mem_bytes = sort_mem * (Size)1024;
/*
* We want to be sure the cost of a sort is never estimated as zero, even
@@ -2488,7 +2488,7 @@ cost_material(Path *path,
Cost startup_cost = input_startup_cost;
Cost run_cost = input_total_cost - input_startup_cost;
double nbytes = relation_byte_size(tuples, width);
- long work_mem_bytes = work_mem * 1024L;
+ double work_mem_bytes = work_mem * (Size)1024;
path->rows = tuples;
@@ -4028,7 +4028,7 @@ final_cost_mergejoin(PlannerInfo *root, MergePath *path,
else if (enable_material && innersortkeys != NIL &&
relation_byte_size(inner_path_rows,
inner_path->pathtarget->width) >
- (work_mem * 1024L))
+ (double)(work_mem * (Size)1024))
path->materialize_inner = true;
else
path->materialize_inner = false;
@@ -4663,7 +4663,7 @@ cost_rescan(PlannerInfo *root, Path *path,
Cost run_cost = cpu_tuple_cost * path->rows;
double nbytes = relation_byte_size(path->rows,
path->pathtarget->width);
- long work_mem_bytes = work_mem * 1024L;
+ double work_mem_bytes = work_mem * (Size)1024;
if (nbytes > work_mem_bytes)
{
@@ -4690,7 +4690,7 @@ cost_rescan(PlannerInfo *root, Path *path,
Cost run_cost = cpu_operator_cost * path->rows;
double nbytes = relation_byte_size(path->rows,
path->pathtarget->width);
- long work_mem_bytes = work_mem * 1024L;
+ double work_mem_bytes = work_mem * (Size)1024;
if (nbytes > work_mem_bytes)
{
@@ -6527,7 +6527,7 @@ compute_bitmap_pages(PlannerInfo *root, RelOptInfo *baserel,
* the bitmap at one time.)
*/
heap_pages = Min(pages_fetched, baserel->pages);
- maxentries = tbm_calculate_entries(work_mem * 1024L);
+ maxentries = (long)tbm_calculate_entries(work_mem * (Size)1024);
if (loop_count > 1)
{
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 79b60df7cf..d35fe8a0f4 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -3643,7 +3643,7 @@ ReorderBufferCheckMemoryLimit(ReorderBuffer *rb)
* haven't exceeded the memory limit.
*/
if (debug_logical_replication_streaming == DEBUG_LOGICAL_REP_STREAMING_BUFFERED &&
- rb->size < logical_decoding_work_mem * 1024L)
+ rb->size < logical_decoding_work_mem * (Size)1024)
return;
/*
@@ -3656,7 +3656,7 @@ ReorderBufferCheckMemoryLimit(ReorderBuffer *rb)
* because a user can reduce the logical_decoding_work_mem to a smaller
* value before the most recent change.
*/
- while (rb->size >= logical_decoding_work_mem * 1024L ||
+ while (rb->size >= logical_decoding_work_mem * (Size)1024 ||
(debug_logical_replication_streaming == DEBUG_LOGICAL_REP_STREAMING_IMMEDIATE &&
rb->size > 0))
{
@@ -3699,7 +3699,7 @@ ReorderBufferCheckMemoryLimit(ReorderBuffer *rb)
}
/* We must be under the memory limit now. */
- Assert(rb->size < logical_decoding_work_mem * 1024L);
+ Assert(rb->size < logical_decoding_work_mem * (Size)1024);
}
diff --git a/src/backend/utils/misc/stack_depth.c b/src/backend/utils/misc/stack_depth.c
index 1c79b919f4..acdf0a155e 100644
--- a/src/backend/utils/misc/stack_depth.c
+++ b/src/backend/utils/misc/stack_depth.c
@@ -26,7 +26,7 @@
int max_stack_depth = 100;
/* max_stack_depth converted to bytes for speed of checking */
-static long max_stack_depth_bytes = 100 * 1024L;
+static Size max_stack_depth_bytes = 100 * (Size)1024;
/*
* Stack base pointer -- initialized by set_stack_base(), which
@@ -158,7 +158,7 @@ check_max_stack_depth(int *newval, void **extra, GucSource source)
void
assign_max_stack_depth(int newval, void *extra)
{
- long newval_bytes = newval * 1024L;
+ Size newval_bytes = newval * (Size)1024;
max_stack_depth_bytes = newval_bytes;
}
diff --git a/src/backend/utils/sort/tuplestore.c b/src/backend/utils/sort/tuplestore.c
index aacec8b799..dcd6424db6 100644
--- a/src/backend/utils/sort/tuplestore.c
+++ b/src/backend/utils/sort/tuplestore.c
@@ -265,7 +265,7 @@ tuplestore_begin_common(int eflags, bool interXact, int maxKBytes)
state->truncated = false;
state->usedDisk = false;
state->maxSpace = 0;
- state->allowedMem = maxKBytes * 1024L;
+ state->allowedMem = maxKBytes * (Size)1024;
state->availMem = state->allowedMem;
state->myfile = NULL;
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index fedbf69eb5..fb86678844 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -62,7 +62,7 @@ typedef struct TBMIterateResult
/* function prototypes in nodes/tidbitmap.c */
-extern TIDBitmap *tbm_create(long maxbytes, dsa_area *dsa);
+extern TIDBitmap *tbm_create(Size maxbytes, dsa_area *dsa);
extern void tbm_free(TIDBitmap *tbm);
extern void tbm_free_shared_area(dsa_area *dsa, dsa_pointer dp);
@@ -84,7 +84,7 @@ extern void tbm_end_private_iterate(TBMPrivateIterator *iterator);
extern void tbm_end_shared_iterate(TBMSharedIterator *iterator);
extern TBMSharedIterator *tbm_attach_shared_iterate(dsa_area *dsa,
dsa_pointer dp);
-extern long tbm_calculate_entries(double maxbytes);
+extern Size tbm_calculate_entries(Size maxbytes);
extern TBMIterator tbm_begin_iterate(TIDBitmap *tbm,
dsa_area *dsa, dsa_pointer dsp);
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 532d6642bb..4492874508 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -18,8 +18,7 @@
/* 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)
--
2.39.5 (Apple Git-154)
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