public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v1 01/12] jit: Instrument function purpose separately, add tracking of modules. 14+ messages / 3 participants [nested] [flat]
* [PATCH v1 01/12] jit: Instrument function purpose separately, add tracking of modules. @ 2019-09-26 18:44 Andres Freund <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Andres Freund @ 2019-09-26 18:44 UTC (permalink / raw) Author: Reviewed-By: Discussion: https://postgr.es/m/ Backpatch: --- src/backend/commands/explain.c | 24 +++++++++++++++++++++++- src/backend/jit/jit.c | 3 +++ src/backend/jit/llvm/llvmjit.c | 2 ++ src/backend/jit/llvm/llvmjit_deform.c | 1 + src/backend/jit/llvm/llvmjit_expr.c | 1 + src/include/jit/jit.h | 11 ++++++++++- 6 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 62fb3434a32..ef65035bfba 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -825,7 +825,26 @@ ExplainPrintJIT(ExplainState *es, int jit_flags, appendStringInfoString(es->str, "JIT:\n"); es->indent += 1; - ExplainPropertyInteger("Functions", NULL, ji->created_functions, es); + /* having to emit code more than once has performance consequences */ + if (ji->created_modules > 1) + ExplainPropertyInteger("Modules", NULL, ji->created_modules, es); + + appendStringInfoSpaces(es->str, es->indent * 2); + appendStringInfo(es->str, "Functions: %zu", ji->created_functions); + if (ji->created_expr_functions > 0 || ji->created_deform_functions) + { + appendStringInfoString(es->str, " ("); + if (ji->created_expr_functions) + { + appendStringInfo(es->str, "%zu for expression evaluation", ji->created_expr_functions); + if (ji->created_deform_functions) + appendStringInfoString(es->str, ", "); + } + if (ji->created_deform_functions) + appendStringInfo(es->str, "%zu for tuple deforming", ji->created_deform_functions); + appendStringInfoChar(es->str, ')'); + } + appendStringInfoChar(es->str, '\n'); appendStringInfoSpaces(es->str, es->indent * 2); appendStringInfo(es->str, "Options: %s %s, %s %s, %s %s, %s %s\n", @@ -851,7 +870,10 @@ ExplainPrintJIT(ExplainState *es, int jit_flags, else { ExplainPropertyInteger("Worker Number", NULL, worker_num, es); + ExplainPropertyInteger("Modules", NULL, ji->created_modules, es); ExplainPropertyInteger("Functions", NULL, ji->created_functions, es); + ExplainPropertyInteger("Expression Functions", NULL, ji->created_expr_functions, es); + ExplainPropertyInteger("Deforming Functions", NULL, ji->created_deform_functions, es); ExplainOpenGroup("Options", "Options", true, es); ExplainPropertyBool("Inlining", jit_flags & PGJIT_INLINE, es); diff --git a/src/backend/jit/jit.c b/src/backend/jit/jit.c index 43e65b1a543..63c709002d8 100644 --- a/src/backend/jit/jit.c +++ b/src/backend/jit/jit.c @@ -186,7 +186,10 @@ jit_compile_expr(struct ExprState *state) void InstrJitAgg(JitInstrumentation *dst, JitInstrumentation *add) { + dst->created_modules += add->created_modules; dst->created_functions += add->created_functions; + dst->created_expr_functions += add->created_expr_functions; + dst->created_deform_functions += add->created_deform_functions; INSTR_TIME_ADD(dst->generation_counter, add->generation_counter); INSTR_TIME_ADD(dst->inlining_counter, add->inlining_counter); INSTR_TIME_ADD(dst->optimization_counter, add->optimization_counter); diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c index 82c4afb7011..5489e118041 100644 --- a/src/backend/jit/llvm/llvmjit.c +++ b/src/backend/jit/llvm/llvmjit.c @@ -212,6 +212,8 @@ llvm_mutable_module(LLVMJitContext *context) context->module = LLVMModuleCreateWithName("pg"); LLVMSetTarget(context->module, llvm_triple); LLVMSetDataLayout(context->module, llvm_layout); + + context->base.instr.created_modules++; } return context->module; diff --git a/src/backend/jit/llvm/llvmjit_deform.c b/src/backend/jit/llvm/llvmjit_deform.c index 835aea83e97..80a85858524 100644 --- a/src/backend/jit/llvm/llvmjit_deform.c +++ b/src/backend/jit/llvm/llvmjit_deform.c @@ -101,6 +101,7 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, mod = llvm_mutable_module(context); funcname = llvm_expand_funcname(context, "deform"); + context->base.instr.created_deform_functions++; /* * Check which columns have to exist, so we don't have to check the row's diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c index 30133634c70..7efc8f23ee3 100644 --- a/src/backend/jit/llvm/llvmjit_expr.c +++ b/src/backend/jit/llvm/llvmjit_expr.c @@ -144,6 +144,7 @@ llvm_compile_expr(ExprState *state) b = LLVMCreateBuilder(); funcname = llvm_expand_funcname(context, "evalexpr"); + context->base.instr.created_expr_functions++; /* Create the signature and function */ { diff --git a/src/include/jit/jit.h b/src/include/jit/jit.h index d879cef20f3..668f965cb0a 100644 --- a/src/include/jit/jit.h +++ b/src/include/jit/jit.h @@ -26,9 +26,18 @@ typedef struct JitInstrumentation { - /* number of emitted functions */ + /* number of modules (i.e. separate optimize / link cycles) created */ + size_t created_modules; + + /* number of functions generated */ size_t created_functions; + /* number of expression evaluation functions generated */ + size_t created_expr_functions; + + /* number of tuple deforming functions generated */ + size_t created_deform_functions; + /* accumulated time to generate code */ instr_time generation_counter; -- 2.23.0.162.gf1d4a28250 --kgppz6muakthis74 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v1-0002-Refactor-explain.c-to-pass-ExprState-down-to-show.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v2 1/8] jit: Instrument function purpose separately, add tracking of modules. @ 2019-09-26 18:44 Andres Freund <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Andres Freund @ 2019-09-26 18:44 UTC (permalink / raw) Author: Reviewed-By: Discussion: https://postgr.es/m/ Backpatch: --- src/backend/commands/explain.c | 24 +++++++++++++++++++++++- src/backend/jit/jit.c | 3 +++ src/backend/jit/llvm/llvmjit.c | 2 ++ src/backend/jit/llvm/llvmjit_deform.c | 1 + src/backend/jit/llvm/llvmjit_expr.c | 1 + src/include/jit/jit.h | 11 ++++++++++- 6 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 62fb3434a32..ef65035bfba 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -825,7 +825,26 @@ ExplainPrintJIT(ExplainState *es, int jit_flags, appendStringInfoString(es->str, "JIT:\n"); es->indent += 1; - ExplainPropertyInteger("Functions", NULL, ji->created_functions, es); + /* having to emit code more than once has performance consequences */ + if (ji->created_modules > 1) + ExplainPropertyInteger("Modules", NULL, ji->created_modules, es); + + appendStringInfoSpaces(es->str, es->indent * 2); + appendStringInfo(es->str, "Functions: %zu", ji->created_functions); + if (ji->created_expr_functions > 0 || ji->created_deform_functions) + { + appendStringInfoString(es->str, " ("); + if (ji->created_expr_functions) + { + appendStringInfo(es->str, "%zu for expression evaluation", ji->created_expr_functions); + if (ji->created_deform_functions) + appendStringInfoString(es->str, ", "); + } + if (ji->created_deform_functions) + appendStringInfo(es->str, "%zu for tuple deforming", ji->created_deform_functions); + appendStringInfoChar(es->str, ')'); + } + appendStringInfoChar(es->str, '\n'); appendStringInfoSpaces(es->str, es->indent * 2); appendStringInfo(es->str, "Options: %s %s, %s %s, %s %s, %s %s\n", @@ -851,7 +870,10 @@ ExplainPrintJIT(ExplainState *es, int jit_flags, else { ExplainPropertyInteger("Worker Number", NULL, worker_num, es); + ExplainPropertyInteger("Modules", NULL, ji->created_modules, es); ExplainPropertyInteger("Functions", NULL, ji->created_functions, es); + ExplainPropertyInteger("Expression Functions", NULL, ji->created_expr_functions, es); + ExplainPropertyInteger("Deforming Functions", NULL, ji->created_deform_functions, es); ExplainOpenGroup("Options", "Options", true, es); ExplainPropertyBool("Inlining", jit_flags & PGJIT_INLINE, es); diff --git a/src/backend/jit/jit.c b/src/backend/jit/jit.c index 43e65b1a543..63c709002d8 100644 --- a/src/backend/jit/jit.c +++ b/src/backend/jit/jit.c @@ -186,7 +186,10 @@ jit_compile_expr(struct ExprState *state) void InstrJitAgg(JitInstrumentation *dst, JitInstrumentation *add) { + dst->created_modules += add->created_modules; dst->created_functions += add->created_functions; + dst->created_expr_functions += add->created_expr_functions; + dst->created_deform_functions += add->created_deform_functions; INSTR_TIME_ADD(dst->generation_counter, add->generation_counter); INSTR_TIME_ADD(dst->inlining_counter, add->inlining_counter); INSTR_TIME_ADD(dst->optimization_counter, add->optimization_counter); diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c index 82c4afb7011..5489e118041 100644 --- a/src/backend/jit/llvm/llvmjit.c +++ b/src/backend/jit/llvm/llvmjit.c @@ -212,6 +212,8 @@ llvm_mutable_module(LLVMJitContext *context) context->module = LLVMModuleCreateWithName("pg"); LLVMSetTarget(context->module, llvm_triple); LLVMSetDataLayout(context->module, llvm_layout); + + context->base.instr.created_modules++; } return context->module; diff --git a/src/backend/jit/llvm/llvmjit_deform.c b/src/backend/jit/llvm/llvmjit_deform.c index 835aea83e97..80a85858524 100644 --- a/src/backend/jit/llvm/llvmjit_deform.c +++ b/src/backend/jit/llvm/llvmjit_deform.c @@ -101,6 +101,7 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, mod = llvm_mutable_module(context); funcname = llvm_expand_funcname(context, "deform"); + context->base.instr.created_deform_functions++; /* * Check which columns have to exist, so we don't have to check the row's diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c index d09324637b9..4ba8c78cbc9 100644 --- a/src/backend/jit/llvm/llvmjit_expr.c +++ b/src/backend/jit/llvm/llvmjit_expr.c @@ -144,6 +144,7 @@ llvm_compile_expr(ExprState *state) b = LLVMCreateBuilder(); funcname = llvm_expand_funcname(context, "evalexpr"); + context->base.instr.created_expr_functions++; /* Create the signature and function */ { diff --git a/src/include/jit/jit.h b/src/include/jit/jit.h index d879cef20f3..668f965cb0a 100644 --- a/src/include/jit/jit.h +++ b/src/include/jit/jit.h @@ -26,9 +26,18 @@ typedef struct JitInstrumentation { - /* number of emitted functions */ + /* number of modules (i.e. separate optimize / link cycles) created */ + size_t created_modules; + + /* number of functions generated */ size_t created_functions; + /* number of expression evaluation functions generated */ + size_t created_expr_functions; + + /* number of tuple deforming functions generated */ + size_t created_deform_functions; + /* accumulated time to generate code */ instr_time generation_counter; -- 2.23.0.385.gbc12974a89 --ga6shgqrocqphdjc Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v2-0002-Refactor-explain.c-to-pass-ExprState-down-to-show.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Direct I/O @ 2023-04-19 17:43 Andres Freund <[email protected]> 2023-08-22 20:48 ` Re: Direct I/O Peter Geoghegan <[email protected]> 0 siblings, 1 reply; 14+ messages in thread From: Andres Freund @ 2023-04-19 17:43 UTC (permalink / raw) To: Robert Haas <[email protected]>; +Cc: Peter Geoghegan <[email protected]>; Greg Stark <[email protected]>; Thomas Munro <[email protected]>; Tom Lane <[email protected]>; Andrew Dunstan <[email protected]>; Dagfinn Ilmari Mannsåker <[email protected]>; Christoph Berg <[email protected]>; [email protected]; pgsql-hackers Hi, On 2023-04-19 13:16:54 -0400, Robert Haas wrote: > On Wed, Apr 19, 2023 at 12:54 PM Andres Freund <[email protected]> wrote: > > Interestingly, I haven't seen that as much in more recent benchmarks as it > > used to. Partially I think because common s_b settings have gotten bigger, I > > guess. But I wonder if we also accidentally improved something else, e.g. by > > pin/unpin-ing the same buffer a bit less frequently. > > I think the problem with the algorithm is pretty fundamental. The rate > of usage count increase is tied to how often we access buffers, and > the rate of usage count decrease is tied to buffer eviction. But a > given workload can have no eviction at all (in which case the usage > counts must converge to 5) or it can evict on every buffer access (in > which case the usage counts must mostly converget to 0, because we'll > be decreasing usage counts at least once per buffer and generally > more). I don't think the "evict on every buffer access" works quite that way - unless you have a completely even access pattern, buffer access frequency will increase usage count much more frequently on some buffers than others. And if you have a completely even access pattern, it's hard to come up with a good cache replacement algorithm... > ISTM that the only way that you can end up with a good mix of > usage counts is if you have a workload that falls into some kind of a > sweet spot where the rate of usage count bumps and the rate of usage > count de-bumps are close enough together that things don't skew all > the way to one end or the other. Bigger s_b might make that more > likely to happen in practice, but it seems like bad algorithm design > on a theoretical level. We should be comparing the frequency of access > of buffers to the frequency of access of other buffers, not to the > frequency of buffer eviction. Or to put the same thing another way, > the average value of the usage count shouldn't suddenly change from 5 > to 1 when the server evicts 1 buffer. I agree that there are fundamental issues with the algorithm. But practically I think the effect of the over-saturation of s_b isn't as severe as one might think: If your miss rate is very low, the occasional bad victim buffer selection won't matter that much. If the miss rate is a bit higher, the likelihood of the usagecount being increased again after being decreased is higher if a buffer is accessed frequently. This is also why I think that larger s_b makes the issues less likely - with larger s_b, it is more likely that frequently accessed buffers are accessed again after the first of the 5 clock sweeps necessary to reduce the usage count. Clearly, with a small-ish s_b and a high replacement rate, that's not going to happen for sufficiently many buffers. But once you have a few GB of s_b, multiple complete sweeps take a while. Most, if not all, buffer replacement algorithms I have seen, don't deal well with "small SB with a huge replacement rate". Most of the fancier algorithms track recency information for buffers that have recently been evicted - but you obviously can't track that to an unlimited degree, IIRC most papers propose that the shadow map to be roughly equal to the buffer pool size. You IMO pretty much need a policy decision on a higher level to improve upon that (e.g. by just deciding that some buffers are sticky, perhaps because they were used first) - but it doesn't matter much, because the miss rate is high enough that the total amount of reads is barely affected by the buffer replacement decisions. Greetings, Andres Freund ^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Direct I/O 2023-04-19 17:43 Re: Direct I/O Andres Freund <[email protected]> @ 2023-08-22 20:48 ` Peter Geoghegan <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Peter Geoghegan @ 2023-08-22 20:48 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: Robert Haas <[email protected]>; Greg Stark <[email protected]>; Thomas Munro <[email protected]>; Tom Lane <[email protected]>; Andrew Dunstan <[email protected]>; Dagfinn Ilmari Mannsåker <[email protected]>; Christoph Berg <[email protected]>; [email protected]; pgsql-hackers On Wed, Apr 19, 2023 at 10:43 AM Andres Freund <[email protected]> wrote: > I don't think the "evict on every buffer access" works quite that way - unless > you have a completely even access pattern, buffer access frequency will > increase usage count much more frequently on some buffers than others. And if > you have a completely even access pattern, it's hard to come up with a good > cache replacement algorithm... My guess is that the most immediate problem in this area is the problem of "correlated references" (to use the term from the LRU-K paper). I gave an example of that here: https://postgr.es/m/CAH2-Wzk7T9K3d9_NY+jEXp2qQGMYoP=gZMoR8q1Cv57SxAw1OA@mail.gmail.com In other words, I think that the most immediate problem may in fact be the tendency of usage_count to get incremented multiple times in response to what is (for all intents and purposes) the same logical page access. Even if it's not as important as I imagine, it still seems likely that verifying that our input information isn't garbage is the logical place to begin work in this general area. It's difficult to be sure about that because it's so hard to look at just one problem in isolation. I suspect that you were right to point out that a larger shared buffers tends to look quite different to a smaller shared buffers. That same factor is going to complicate any analysis of the specific problem that I've highlighted (to say nothing of the way that contention complicates the picture). There is an interesting paper that compared the hit rates seen for TPC-C to TPC-E on relatively modern hardware: https://www.cs.cmu.edu/~chensm/papers/TPCE-sigmod-record10.pdf It concludes that the buffer misses for each workload look rather similar, past a certain point (past a certain buffer pool size): both workloads have cache misses that seem totally random. The access patterns may be very different, but that doesn't necessarily have any visible effect on buffer misses. At least provided that you make certain modest assumptions about buffer pool size, relative to working set size. The most sophisticated cache management algorithms (like ARC) work by maintaining metadata about recently evicted buffers, which is used to decide whether to favor recency over frequency. If you work backwards then it follows that having cache misses that look completely random is desirable, and perhaps even something to work towards. What you really don't want is a situation where the same small minority of pages keep getting ping-ponged into and out of the buffer pool, without ever settling, even though the buffer cache is large enough that that's possible in principle. That pathological profile is the furthest possible thing from random. With smaller shared_buffers, it's perhaps inevitable that buffer cache misses are random, and so I'd expect that managing the problem of contention will tend to matter most. With larger shared_buffers it isn't inevitable at all, so the quality of the cache eviction scheme is likely to matter quite a bit more. -- Peter Geoghegan ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v45 2/8] Do not dereference varattrib_4b in VARSIZE_4B. @ 2026-03-11 13:53 Antonin Houska <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Antonin Houska @ 2026-03-11 13:53 UTC (permalink / raw) Since VARSIZE_ANY() may call VARSIZE_4B(), it's possible that the compiler (when invoked with -Warray-bounds) complains if the argument of VARSIZE_ANY() is actually smaller than what VARSIZE_4B() expects. This patch adjusts the VARSIZE_4B() macro so that it does not have to dereference the varattrib_4b structure. We assume that varlena value always starts with the length word. The problem does not exist in the tree at the moment since the current users of VARSIZE_ANY() pass a pointer to a dynamically allocated memory, so the compiler has no idea about the memory available. However, in an upcoming patch, it makes sense to pass a pointer to a local variable of "varlena" type. In such a case, the compiler warning might appear because sizeof(varlena) is lower than sizeof(varattrib_4b). --- src/include/varatt.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/varatt.h b/src/include/varatt.h index 000bdf33b92..31063e5d4f1 100644 --- a/src/include/varatt.h +++ b/src/include/varatt.h @@ -207,7 +207,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - (((const varattrib_4b *) (PTR))->va_4byte.va_header & 0x3FFFFFFF) + (*((const uint32 *) (PTR)) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ (((const varattrib_1b *) (PTR))->va_header & 0x7F) #define VARTAG_1B_E(PTR) \ @@ -240,7 +240,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - ((((const varattrib_4b *) (PTR))->va_4byte.va_header >> 2) & 0x3FFFFFFF) + ((*((const uint32 *) (PTR)) >> 2) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ ((((const varattrib_1b *) (PTR))->va_header >> 1) & 0x7F) #define VARTAG_1B_E(PTR) \ -- 2.47.3 --cldir6umqisr673d Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v45-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v44b 2/4] Do not dereference varattrib_4b in VARSIZE_4B. @ 2026-03-11 13:53 Antonin Houska <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Antonin Houska @ 2026-03-11 13:53 UTC (permalink / raw) Since VARSIZE_ANY() may call VARSIZE_4B(), it's possible that the compiler (when invoked with -Warray-bounds) complains if the argument of VARSIZE_ANY() is actually smaller than what VARSIZE_4B() expects. This patch adjusts the VARSIZE_4B() macro so that it does not have to dereference the varattrib_4b structure. We assume that varlena value always starts with the length word. The problem does not exist in the tree at the moment since the current users of VARSIZE_ANY() pass a pointer to a dynamically allocated memory, so the compiler has no idea about the memory available. However, in an upcoming patch, it makes sense to pass a pointer to a local variable of "varlena" type. In such a case, the compiler warning might appear because sizeof(varlena) is lower than sizeof(varattrib_4b). --- src/include/varatt.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/varatt.h b/src/include/varatt.h index 000bdf33b92..31063e5d4f1 100644 --- a/src/include/varatt.h +++ b/src/include/varatt.h @@ -207,7 +207,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - (((const varattrib_4b *) (PTR))->va_4byte.va_header & 0x3FFFFFFF) + (*((const uint32 *) (PTR)) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ (((const varattrib_1b *) (PTR))->va_header & 0x7F) #define VARTAG_1B_E(PTR) \ @@ -240,7 +240,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - ((((const varattrib_4b *) (PTR))->va_4byte.va_header >> 2) & 0x3FFFFFFF) + ((*((const uint32 *) (PTR)) >> 2) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ ((((const varattrib_1b *) (PTR))->va_header >> 1) & 0x7F) #define VARTAG_1B_E(PTR) \ -- 2.47.3 --fot5plqmb33ey33c Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v44b-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v45 2/8] Do not dereference varattrib_4b in VARSIZE_4B. @ 2026-03-11 13:53 Antonin Houska <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Antonin Houska @ 2026-03-11 13:53 UTC (permalink / raw) Since VARSIZE_ANY() may call VARSIZE_4B(), it's possible that the compiler (when invoked with -Warray-bounds) complains if the argument of VARSIZE_ANY() is actually smaller than what VARSIZE_4B() expects. This patch adjusts the VARSIZE_4B() macro so that it does not have to dereference the varattrib_4b structure. We assume that varlena value always starts with the length word. The problem does not exist in the tree at the moment since the current users of VARSIZE_ANY() pass a pointer to a dynamically allocated memory, so the compiler has no idea about the memory available. However, in an upcoming patch, it makes sense to pass a pointer to a local variable of "varlena" type. In such a case, the compiler warning might appear because sizeof(varlena) is lower than sizeof(varattrib_4b). --- src/include/varatt.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/varatt.h b/src/include/varatt.h index 000bdf33b92..31063e5d4f1 100644 --- a/src/include/varatt.h +++ b/src/include/varatt.h @@ -207,7 +207,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - (((const varattrib_4b *) (PTR))->va_4byte.va_header & 0x3FFFFFFF) + (*((const uint32 *) (PTR)) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ (((const varattrib_1b *) (PTR))->va_header & 0x7F) #define VARTAG_1B_E(PTR) \ @@ -240,7 +240,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - ((((const varattrib_4b *) (PTR))->va_4byte.va_header >> 2) & 0x3FFFFFFF) + ((*((const uint32 *) (PTR)) >> 2) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ ((((const varattrib_1b *) (PTR))->va_header >> 1) & 0x7F) #define VARTAG_1B_E(PTR) \ -- 2.47.3 --cldir6umqisr673d Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v45-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v43 2/7] Do not dereference varattrib_4b in VARSIZE_4B. @ 2026-03-11 13:53 Antonin Houska <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Antonin Houska @ 2026-03-11 13:53 UTC (permalink / raw) Since VARSIZE_ANY() may call VARSIZE_4B(), it's possible that the compiler (when invoked with -Warray-bounds) complains if the argument of VARSIZE_ANY() is actually smaller than what VARSIZE_4B() expects. This patch adjusts the VARSIZE_4B() macro so that it does not have to dereference the varattrib_4b structure. We assume that varlena value always starts with the length word. The problem does not exist in the tree at the moment since the current users of VARSIZE_ANY() pass a pointer to a dynamically allocated memory, so the compiler has no idea about the memory available. However, in an upcoming patch, it makes sense to pass a pointer to a local variable of "varlena" type. In such a case, the compiler warning might appear because sizeof(varlena) is lower than sizeof(varattrib_4b). --- src/include/varatt.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/varatt.h b/src/include/varatt.h index 000bdf33b92..31063e5d4f1 100644 --- a/src/include/varatt.h +++ b/src/include/varatt.h @@ -207,7 +207,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - (((const varattrib_4b *) (PTR))->va_4byte.va_header & 0x3FFFFFFF) + (*((const uint32 *) (PTR)) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ (((const varattrib_1b *) (PTR))->va_header & 0x7F) #define VARTAG_1B_E(PTR) \ @@ -240,7 +240,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - ((((const varattrib_4b *) (PTR))->va_4byte.va_header >> 2) & 0x3FFFFFFF) + ((*((const uint32 *) (PTR)) >> 2) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ ((((const varattrib_1b *) (PTR))->va_header >> 1) & 0x7F) #define VARTAG_1B_E(PTR) \ -- 2.47.3 --nzintiedl6o4kcyp Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v43-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v43 2/7] Do not dereference varattrib_4b in VARSIZE_4B. @ 2026-03-11 13:53 Antonin Houska <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Antonin Houska @ 2026-03-11 13:53 UTC (permalink / raw) Since VARSIZE_ANY() may call VARSIZE_4B(), it's possible that the compiler (when invoked with -Warray-bounds) complains if the argument of VARSIZE_ANY() is actually smaller than what VARSIZE_4B() expects. This patch adjusts the VARSIZE_4B() macro so that it does not have to dereference the varattrib_4b structure. We assume that varlena value always starts with the length word. The problem does not exist in the tree at the moment since the current users of VARSIZE_ANY() pass a pointer to a dynamically allocated memory, so the compiler has no idea about the memory available. However, in an upcoming patch, it makes sense to pass a pointer to a local variable of "varlena" type. In such a case, the compiler warning might appear because sizeof(varlena) is lower than sizeof(varattrib_4b). --- src/include/varatt.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/varatt.h b/src/include/varatt.h index 000bdf33b92..31063e5d4f1 100644 --- a/src/include/varatt.h +++ b/src/include/varatt.h @@ -207,7 +207,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - (((const varattrib_4b *) (PTR))->va_4byte.va_header & 0x3FFFFFFF) + (*((const uint32 *) (PTR)) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ (((const varattrib_1b *) (PTR))->va_header & 0x7F) #define VARTAG_1B_E(PTR) \ @@ -240,7 +240,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - ((((const varattrib_4b *) (PTR))->va_4byte.va_header >> 2) & 0x3FFFFFFF) + ((*((const uint32 *) (PTR)) >> 2) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ ((((const varattrib_1b *) (PTR))->va_header >> 1) & 0x7F) #define VARTAG_1B_E(PTR) \ -- 2.47.3 --nzintiedl6o4kcyp Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v43-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v44b 2/4] Do not dereference varattrib_4b in VARSIZE_4B. @ 2026-03-11 13:53 Antonin Houska <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Antonin Houska @ 2026-03-11 13:53 UTC (permalink / raw) Since VARSIZE_ANY() may call VARSIZE_4B(), it's possible that the compiler (when invoked with -Warray-bounds) complains if the argument of VARSIZE_ANY() is actually smaller than what VARSIZE_4B() expects. This patch adjusts the VARSIZE_4B() macro so that it does not have to dereference the varattrib_4b structure. We assume that varlena value always starts with the length word. The problem does not exist in the tree at the moment since the current users of VARSIZE_ANY() pass a pointer to a dynamically allocated memory, so the compiler has no idea about the memory available. However, in an upcoming patch, it makes sense to pass a pointer to a local variable of "varlena" type. In such a case, the compiler warning might appear because sizeof(varlena) is lower than sizeof(varattrib_4b). --- src/include/varatt.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/varatt.h b/src/include/varatt.h index 000bdf33b92..31063e5d4f1 100644 --- a/src/include/varatt.h +++ b/src/include/varatt.h @@ -207,7 +207,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - (((const varattrib_4b *) (PTR))->va_4byte.va_header & 0x3FFFFFFF) + (*((const uint32 *) (PTR)) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ (((const varattrib_1b *) (PTR))->va_header & 0x7F) #define VARTAG_1B_E(PTR) \ @@ -240,7 +240,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - ((((const varattrib_4b *) (PTR))->va_4byte.va_header >> 2) & 0x3FFFFFFF) + ((*((const uint32 *) (PTR)) >> 2) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ ((((const varattrib_1b *) (PTR))->va_header >> 1) & 0x7F) #define VARTAG_1B_E(PTR) \ -- 2.47.3 --fot5plqmb33ey33c Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v44b-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH 2/7] Do not dereference varattrib_4b in VARSIZE_4B. @ 2026-03-11 13:53 Antonin Houska <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Antonin Houska @ 2026-03-11 13:53 UTC (permalink / raw) Since VARSIZE_ANY() may call VARSIZE_4B(), it's possible that the compiler (when invoked with -Warray-bounds) complains if the argument of VARSIZE_ANY() is actually smaller than what VARSIZE_4B() expects. This patch adjusts the VARSIZE_4B() macro so that it does not have to dereference the varattrib_4b structure. We assume that varlena value always starts with the length word. The problem does not exist in the tree at the moment since the current users of VARSIZE_ANY() pass a pointer to a dynamically allocated memory, so the compiler has no idea about the memory available. However, in an upcoming patch, it makes sense to pass a pointer to a local variable of "varlena" type. In such a case, the compiler warning might appear because sizeof(varlena) is lower than sizeof(varattrib_4b). --- src/include/varatt.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/varatt.h b/src/include/varatt.h index 000bdf33b92..31063e5d4f1 100644 --- a/src/include/varatt.h +++ b/src/include/varatt.h @@ -207,7 +207,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - (((const varattrib_4b *) (PTR))->va_4byte.va_header & 0x3FFFFFFF) + (*((const uint32 *) (PTR)) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ (((const varattrib_1b *) (PTR))->va_header & 0x7F) #define VARTAG_1B_E(PTR) \ @@ -240,7 +240,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - ((((const varattrib_4b *) (PTR))->va_4byte.va_header >> 2) & 0x3FFFFFFF) + ((*((const uint32 *) (PTR)) >> 2) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ ((((const varattrib_1b *) (PTR))->va_header >> 1) & 0x7F) #define VARTAG_1B_E(PTR) \ -- 2.47.3 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v42-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v44 02/10] Do not dereference varattrib_4b in VARSIZE_4B. @ 2026-03-11 13:53 Antonin Houska <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Antonin Houska @ 2026-03-11 13:53 UTC (permalink / raw) Since VARSIZE_ANY() may call VARSIZE_4B(), it's possible that the compiler (when invoked with -Warray-bounds) complains if the argument of VARSIZE_ANY() is actually smaller than what VARSIZE_4B() expects. This patch adjusts the VARSIZE_4B() macro so that it does not have to dereference the varattrib_4b structure. We assume that varlena value always starts with the length word. The problem does not exist in the tree at the moment since the current users of VARSIZE_ANY() pass a pointer to a dynamically allocated memory, so the compiler has no idea about the memory available. However, in an upcoming patch, it makes sense to pass a pointer to a local variable of "varlena" type. In such a case, the compiler warning might appear because sizeof(varlena) is lower than sizeof(varattrib_4b). --- src/include/varatt.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/varatt.h b/src/include/varatt.h index 000bdf33b92..31063e5d4f1 100644 --- a/src/include/varatt.h +++ b/src/include/varatt.h @@ -207,7 +207,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - (((const varattrib_4b *) (PTR))->va_4byte.va_header & 0x3FFFFFFF) + (*((const uint32 *) (PTR)) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ (((const varattrib_1b *) (PTR))->va_header & 0x7F) #define VARTAG_1B_E(PTR) \ @@ -240,7 +240,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - ((((const varattrib_4b *) (PTR))->va_4byte.va_header >> 2) & 0x3FFFFFFF) + ((*((const uint32 *) (PTR)) >> 2) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ ((((const varattrib_1b *) (PTR))->va_header >> 1) & 0x7F) #define VARTAG_1B_E(PTR) \ -- 2.47.3 --gwom7bl7ogtszo4k Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v44-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH 2/7] Do not dereference varattrib_4b in VARSIZE_4B. @ 2026-03-11 13:53 Antonin Houska <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Antonin Houska @ 2026-03-11 13:53 UTC (permalink / raw) Since VARSIZE_ANY() may call VARSIZE_4B(), it's possible that the compiler (when invoked with -Warray-bounds) complains if the argument of VARSIZE_ANY() is actually smaller than what VARSIZE_4B() expects. This patch adjusts the VARSIZE_4B() macro so that it does not have to dereference the varattrib_4b structure. We assume that varlena value always starts with the length word. The problem does not exist in the tree at the moment since the current users of VARSIZE_ANY() pass a pointer to a dynamically allocated memory, so the compiler has no idea about the memory available. However, in an upcoming patch, it makes sense to pass a pointer to a local variable of "varlena" type. In such a case, the compiler warning might appear because sizeof(varlena) is lower than sizeof(varattrib_4b). --- src/include/varatt.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/varatt.h b/src/include/varatt.h index 000bdf33b92..31063e5d4f1 100644 --- a/src/include/varatt.h +++ b/src/include/varatt.h @@ -207,7 +207,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - (((const varattrib_4b *) (PTR))->va_4byte.va_header & 0x3FFFFFFF) + (*((const uint32 *) (PTR)) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ (((const varattrib_1b *) (PTR))->va_header & 0x7F) #define VARTAG_1B_E(PTR) \ @@ -240,7 +240,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - ((((const varattrib_4b *) (PTR))->va_4byte.va_header >> 2) & 0x3FFFFFFF) + ((*((const uint32 *) (PTR)) >> 2) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ ((((const varattrib_1b *) (PTR))->va_header >> 1) & 0x7F) #define VARTAG_1B_E(PTR) \ -- 2.47.3 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v42-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v44 02/10] Do not dereference varattrib_4b in VARSIZE_4B. @ 2026-03-11 13:53 Antonin Houska <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Antonin Houska @ 2026-03-11 13:53 UTC (permalink / raw) Since VARSIZE_ANY() may call VARSIZE_4B(), it's possible that the compiler (when invoked with -Warray-bounds) complains if the argument of VARSIZE_ANY() is actually smaller than what VARSIZE_4B() expects. This patch adjusts the VARSIZE_4B() macro so that it does not have to dereference the varattrib_4b structure. We assume that varlena value always starts with the length word. The problem does not exist in the tree at the moment since the current users of VARSIZE_ANY() pass a pointer to a dynamically allocated memory, so the compiler has no idea about the memory available. However, in an upcoming patch, it makes sense to pass a pointer to a local variable of "varlena" type. In such a case, the compiler warning might appear because sizeof(varlena) is lower than sizeof(varattrib_4b). --- src/include/varatt.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/varatt.h b/src/include/varatt.h index 000bdf33b92..31063e5d4f1 100644 --- a/src/include/varatt.h +++ b/src/include/varatt.h @@ -207,7 +207,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - (((const varattrib_4b *) (PTR))->va_4byte.va_header & 0x3FFFFFFF) + (*((const uint32 *) (PTR)) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ (((const varattrib_1b *) (PTR))->va_header & 0x7F) #define VARTAG_1B_E(PTR) \ @@ -240,7 +240,7 @@ typedef struct /* VARSIZE_4B() should only be used on known-aligned data */ #define VARSIZE_4B(PTR) \ - ((((const varattrib_4b *) (PTR))->va_4byte.va_header >> 2) & 0x3FFFFFFF) + ((*((const uint32 *) (PTR)) >> 2) & 0x3FFFFFFF) #define VARSIZE_1B(PTR) \ ((((const varattrib_1b *) (PTR))->va_header >> 1) & 0x7F) #define VARTAG_1B_E(PTR) \ -- 2.47.3 --gwom7bl7ogtszo4k Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v44-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
end of thread, other threads:[~2026-03-11 13:53 UTC | newest] Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2019-09-26 18:44 [PATCH v1 01/12] jit: Instrument function purpose separately, add tracking of modules. Andres Freund <[email protected]> 2019-09-26 18:44 [PATCH v2 1/8] jit: Instrument function purpose separately, add tracking of modules. Andres Freund <[email protected]> 2023-04-19 17:43 Re: Direct I/O Andres Freund <[email protected]> 2023-08-22 20:48 ` Re: Direct I/O Peter Geoghegan <[email protected]> 2026-03-11 13:53 [PATCH v45 2/8] Do not dereference varattrib_4b in VARSIZE_4B. Antonin Houska <[email protected]> 2026-03-11 13:53 [PATCH v44b 2/4] Do not dereference varattrib_4b in VARSIZE_4B. Antonin Houska <[email protected]> 2026-03-11 13:53 [PATCH v45 2/8] Do not dereference varattrib_4b in VARSIZE_4B. Antonin Houska <[email protected]> 2026-03-11 13:53 [PATCH v43 2/7] Do not dereference varattrib_4b in VARSIZE_4B. Antonin Houska <[email protected]> 2026-03-11 13:53 [PATCH v43 2/7] Do not dereference varattrib_4b in VARSIZE_4B. Antonin Houska <[email protected]> 2026-03-11 13:53 [PATCH v44b 2/4] Do not dereference varattrib_4b in VARSIZE_4B. Antonin Houska <[email protected]> 2026-03-11 13:53 [PATCH 2/7] Do not dereference varattrib_4b in VARSIZE_4B. Antonin Houska <[email protected]> 2026-03-11 13:53 [PATCH v44 02/10] Do not dereference varattrib_4b in VARSIZE_4B. Antonin Houska <[email protected]> 2026-03-11 13:53 [PATCH 2/7] Do not dereference varattrib_4b in VARSIZE_4B. Antonin Houska <[email protected]> 2026-03-11 13:53 [PATCH v44 02/10] Do not dereference varattrib_4b in VARSIZE_4B. Antonin Houska <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox