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