On 11.12.2025 06:33, Michael Paquier wrote: > On Thu, Dec 11, 2025 at 08:01:49AM +0900, Michael Paquier wrote: >> I still need to get through the remaining dubious changes you have >> posted, including the llvm one that was wrong. It seems like some of >> these things warrant a backpatch. > > I have been looking at the rest of these changes with some -O2, and I > have been puzzled by the differences in hstore_gin.c and hstore_op.c. > In all these cases we generate more instructions with the patch than > without the patch. Anyway, I assume that these are the ones that > matter: > entries = (Datum *) palloc(sizeof(Datum) * 2 * count); > out_datums = palloc_array(Datum, count * 2);
Yeah, in a few cases the different bracketing has some surprisingly big influence on the generated assmebly. > pg_trgm.c was less puzzling. For example: > - trg1 = (trgm *) palloc(sizeof(trgm) * (slen1 / 2 + 1) * 3); > + trg1 = palloc_array(trgm, (slen1 / 2 + 1) * 3); > This one leads to something like that before vs after: > < 1418: 83 c0 01 add eax,0x1 >> 1418: 8d 44 40 03 lea eax,[rax+rax*2+0x3] > > In execPartition.c and partprune.c, as far as I can see we are cutting > a few mov, leading to less instructions overall. > > For mvdistinct.c, we are cutting things overall. I am seeing less. > > All the fuzz in postgres_fdw.c is caused by this one: > - p_values = (const char **) palloc(sizeof(char *) * fmstate->p_nums > * numSlots); > + p_values = palloc_array(const char *, fmstate->p_nums * numSlots); > > bufmgr.c did not matter, same before and after. > > I am not completely sure about the one in fuzzystrmatch, I would need > to study more the metaphone code. :) Actually, the change in fuzzystrmatch.c is incorrect. With the new code, palloc(0) will be called for strlen(word) == 0. Then no space for the null-terminator byte will be left. Even if palloc(0) returns memory that can be dereferenced, I don't think it's a good idea to rely on that. > One formula in llvmjit_expr.c has been wrong since v10, so I have > backpatched a fix for it. > > In pg_buffercache_pages.c, the difference is here: > - os_page_status = palloc(sizeof(uint64) * os_page_count); > + os_page_status = palloc_array(int, os_page_count); > Your formula is correct, the previous one was not by using a uint64. > So it allocated twice too much memory. Backpatched this one down to > v18. > > And that should close this thread, at least from my side.. Thanks Michael for taking care of this patch! -- David Geier
