v.popoli...@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
