On Thu, Oct 20, 2022 at 1:55 AM Andres Freund wrote:
>
> Hi,
>
> On 2022-10-11 10:21:17 +0700, John Naylor wrote:
> > On Tue, Oct 11, 2022 at 5:31 AM David Rowley
wrote:
> > >
> > > The proposed patches in [1] do aim to make additional usages of the
> > > slab allocator, and I have a feeling
Hi,
On 2022-10-11 10:21:17 +0700, John Naylor wrote:
> On Tue, Oct 11, 2022 at 5:31 AM David Rowley wrote:
> >
> > The proposed patches in [1] do aim to make additional usages of the
> > slab allocator, and I have a feeling that we'll want to fix the
> > performance of slab.c before those.
On Tue, Oct 11, 2022 at 5:31 AM David Rowley wrote:
>
> The proposed patches in [1] do aim to make additional usages of the
> slab allocator, and I have a feeling that we'll want to fix the
> performance of slab.c before those. Perhaps the Asserts are a better
> option if we're to get the
David Rowley writes:
> The main reason I brought it up was that only yesterday I was looking
> into fixing the slowness of the Slab allocator. It's currently quite
> far behind the performance of both generation.c and aset.c and it
> would be very nice to bring it up to at least be on-par with
On Tue, 11 Oct 2022 at 10:07, Tom Lane wrote:
> Yeah, slab.c hasn't any distinction between large and small chunks,
> so we have to just pick one policy or the other. I'd hoped to get
> away with the more robust runtime test on the basis that slab allocation
> is not used so much that this'd
David Rowley writes:
> Looking at your changes to SlabFree(), I don't really think that
> change is well aligned to the newly proposed policy. My understanding
> of the rationale behind this policy is that large chunks get malloced
> and will be slower anyway, so the elog(ERROR) is less overhead
On Tue, 11 Oct 2022 at 08:35, Tom Lane wrote:
> Hearing no comments on that, I decided that a good policy would be
> to use Asserts in the paths dealing with small chunks but test-and-elog
> in the paths dealing with large chunks.
This seems like a good policy. I think it's good to get at least
I wrote:
> What I am mainly wondering about at this point is whether Asserts
> are good enough or we should use actual test-and-elog checks for
> these things.
Hearing no comments on that, I decided that a good policy would be
to use Asserts in the paths dealing with small chunks but
So I pushed that, but I don't feel that we're out of the woods yet.
As I mentioned at [1], while testing this stuff I hit a case where
aset.c will try to wipe_mem practically the entire address space after
being asked to pfree() an invalid pointer. The specific reproducer
isn't too interesting
David Rowley writes:
> On Fri, 7 Oct 2022 at 12:35, Tom Lane wrote:
>> Which leaves me with the attached proposed wording.
> No objections here.
Cool, I'll push in a little bit.
> With these comments I'd be using slot MCTX_UNUSED4_ID first, then I'd
> probably be looking at MCTX_UNUSED5_ID
On Fri, 7 Oct 2022 at 12:35, Tom Lane wrote:
> Which leaves me with the attached proposed wording.
No objections here.
With these comments I'd be using slot MCTX_UNUSED4_ID first, then I'd
probably be looking at MCTX_UNUSED5_ID after adjusting wipe_mem to do
something other than setting bytes
I wrote:
> So I'm still inclined to leave 001 and 010 both unused, but the
> reason why is different than I thought before.
Which leaves me with the attached proposed wording.
regards, tom lane
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
I wrote:
> FreeBSD 13.0, arm64: Usually the low-order nibble is or ,
> but for some smaller values of N it sometimes comes up as 0010.
> NetBSD 9.2, amd64: results similar to FreeBSD.
I looked into NetBSD's malloc.c, and what I discovered is that
their implementation doesn't have any
On Fri, 7 Oct 2022 at 10:57, Tom Lane wrote:
> I poked at this some more by creating a function that intentionally
> does pfree(malloc(N)) for various values of N.
>
> RHEL8, x86_64: the low-order nibble of the header is consistently 0001.
>
> macOS 12.6, arm64: the low-order nibble is
I wrote:
> I also avoided using 001: based on my work with converting guc.c to use
> palloc [1], it seems that pfree'ing a malloc-provided pointer is likely
> to see 001 a lot, at least on 64-bit glibc platforms.
I poked at this some more by creating a function that intentionally
does
David Rowley writes:
> However, maybe you've left it this way as you feel it's a decision
> that must be made in the future, perhaps based on how difficult it
> would be to free up another bit?
Yeah, pretty much. I think it'll be a long time before we run out
of memory context IDs, and it's
On Fri, 7 Oct 2022 at 09:05, Tom Lane wrote:
>
> Here's a v2 incorporating discussed changes.
>
> In reordering enum MemoryContextMethodID, I arranged to avoid using
> 000 and 111 as valid IDs, since those bit patterns will appear in
> zeroed and wipe_mem'd memory respectively. Those should
Here's a v2 incorporating discussed changes.
In reordering enum MemoryContextMethodID, I arranged to avoid using
000 and 111 as valid IDs, since those bit patterns will appear in
zeroed and wipe_mem'd memory respectively. Those should probably be
more-or-less-permanent exclusions, so I added
Andres Freund writes:
> On 2022-10-06 15:10:44 -0400, Tom Lane wrote:
>> Andres Freund writes:
>>> Maybe worth printing the method ID as well?
>> I doubt it'd be useful.
> I was thinking it could be useful to see whether the bits are likely to be the
> result of wipe_mem(). But I guess for
Hi,
On 2022-10-06 15:10:44 -0400, Tom Lane wrote:
> Andres Freund writes:
> >> + elog(ERROR, "pfree called with invalid pointer %p", pointer);
>
> > Maybe worth printing the method ID as well?
>
> I doubt it'd be useful.
I was thinking it could be useful to see whether the bits are likely to
Andres Freund writes:
> Yea, that makes sense. I wouldn't get rid of the MAXALIGN Assert though - it's
> not replaced by the the unused mcxt stuff afaics.
OK.
>> +elog(ERROR, "pfree called with invalid pointer %p", pointer);
> Maybe worth printing the method ID as well?
I doubt it'd be
Hi,
On 2022-10-06 14:19:21 -0400, Tom Lane wrote:
> One more thing: based on what I saw in working with my pending guc.c
> changes, the assertions in GetMemoryChunkMethodID are largely useless
> for detecting bogus pointers. I think we should do something more
> like the attached, which will
One more thing: based on what I saw in working with my pending guc.c
changes, the assertions in GetMemoryChunkMethodID are largely useless
for detecting bogus pointers. I think we should do something more
like the attached, which will result in a clean failure if the method
ID bits are invalid.
David Rowley writes:
> I did see that PostGIS does use
> MemoryContextContains(), though I didn't look at their code to figure
> out if they're always passing it a pointer to an allocated chunk.
As far as I can tell from a cursory look, they should be able to use
the GetMemoryChunkContext
David Rowley writes:
> On Wed, 5 Oct 2022 at 04:55, Tom Lane wrote:
>> After studying the existing usages of MemoryContextContains, I think
>> there is a better answer, which is to just nuke them.
> I was under the impression you wanted to keep that function around in
> cassert builds for some
On Wed, 5 Oct 2022 at 04:55, Tom Lane wrote:
> After studying the existing usages of MemoryContextContains, I think
> there is a better answer, which is to just nuke them.
I was under the impression you wanted to keep that function around in
cassert builds for some of the guc.c changes you were
I wrote:
> I think what we should look at is extending the aggregate/window
> function APIs so that such functions can report where they put their
> output, and then we can nuke MemoryContextContains(), with the
> code code set up to assume that it has to copy if the called function
> didn't
Em ter., 4 de out. de 2022 às 08:29, Ranier Vilela
escreveu:
> Em ter., 4 de out. de 2022 às 05:36, David Rowley
> escreveu:
>
>> On Tue, 4 Oct 2022 at 13:35, Ranier Vilela wrote:
>> > Revisiting my work on reducing memory consumption, I found this patch
>> left out.
>> > I'm not sure I can
Em ter., 4 de out. de 2022 às 05:36, David Rowley
escreveu:
> On Tue, 4 Oct 2022 at 13:35, Ranier Vilela wrote:
> > Revisiting my work on reducing memory consumption, I found this patch
> left out.
> > I'm not sure I can help.
> > But basically I was able to write and read the block size, in
On Tue, 4 Oct 2022 at 13:35, Ranier Vilela wrote:
> Revisiting my work on reducing memory consumption, I found this patch left
> out.
> I'm not sure I can help.
> But basically I was able to write and read the block size, in the chunk.
> Could it be the case of writing and reading the context
On Thu, 29 Sept 2022 at 18:30, David Rowley
wrote:
> Does anyone have any opinions on this?
Hi,
Revisiting my work on reducing memory consumption, I found this patch left
out.
I'm not sure I can help.
But basically I was able to write and read the block size, in the chunk.
Could it be the case
David Rowley writes:
> Andres did mention to me off-list about perhaps adding a boolean field
> to FunctionCallInfoBaseData to indicate if the return value can be
> assumed to be in CurrentMemoryContext. I feel like that might be
> quite a bit of work to go and change all functions to ensure
On Thu, 29 Sept 2022 at 18:30, David Rowley wrote:
> Does anyone have any opinions on this?
I by no means think I've nailed the fix in
other_ideas_to_fix_MemoryContextContains.patch, so it would be good to
see if anyone else has any new ideas on how to solve this issue.
Andres did mention to me
On Tue, 27 Sept 2022 at 11:28, David Rowley wrote:
> Maybe we could remove the datumCopy() from eval_windowfunction() and
> also document that a window function when returning a non-byval type,
> must allocate the Datum in either ps_ExprContext's
> ecxt_per_tuple_memory or ecxt_per_query_memory.
On Tue, 20 Sept 2022 at 13:23, Tom Lane wrote:
>
> David Rowley writes:
> > Aside from that, I don't have any ideas on how to get rid of the
> > possible additional datumCopy() from non-Var arguments to these window
> > functions. Should we just suffer it? It's quite likely that most
> >
On Tue, 20 Sept 2022 at 17:23, Tom Lane wrote:
> "Broken" is a strong claim. There's reason to think it could fail
> in the back branches, but little evidence that it actually has failed
> in the field.
I've posted some code to the security list that shows that I can get
MemoryContextContains()
David Rowley writes:
> On Tue, 20 Sept 2022 at 13:23, Tom Lane wrote:
>> ... but I'm completely not satisfied with the current
>> situation in HEAD.
> Maybe you've forgotten that MemoryContextContains() is broken in the
> back branches or just don't think it is broken?
"Broken" is a strong
On Tue, 20 Sept 2022 at 13:23, Tom Lane wrote:
>
> David Rowley writes:
> > Aside from that, I don't have any ideas on how to get rid of the
> > possible additional datumCopy() from non-Var arguments to these window
> > functions. Should we just suffer it? It's quite likely that most
> >
David Rowley writes:
> Aside from that, I don't have any ideas on how to get rid of the
> possible additional datumCopy() from non-Var arguments to these window
> functions. Should we just suffer it? It's quite likely that most
> arguments to these functions are plain Vars anyway.
No, we
On Tue, 13 Sept 2022 at 20:27, David Rowley wrote:
> I see that one of the drawbacks from not using MemoryContextContains()
> is that window functions such as lead(), lag(), first_value(),
> last_value() and nth_value() may now do the datumCopy() when it's not
> needed. For example, with a window
On Fri, 9 Sept 2022 at 11:33, David Rowley wrote:
> I really think the Assert only form of MemoryContextContains() is the
> best move, and if it's doing Assert only, then we can do the
> loop-over-the-blocks idea as you described and I drafted in [1].
>
> If the need comes up that we're certain
On Fri, 9 Sept 2022 at 10:53, Tom Lane wrote:
> I hate to give up MemoryContextContains altogether. The assertions
> that David nuked in b76fb6c2a had some value I think,
Those can be put back if we decide to keep MemoryContextContains.
Those newly added Asserts just temporarily had to go due
Andres Freund writes:
> On 2022-09-08 14:10:36 -0400, Tom Lane wrote:
>> No, I don't think we can get away with that. See int8inc() for a
>> counterexample.
> What I was suggesting a bit below the bit quoted above, was that we'd copy
> whenever there's no finalfunc or if the finalfunc doesn't
Hi,
On 2022-09-08 14:10:36 -0400, Tom Lane wrote:
> Andres Freund writes:
> > If there is a finalfunc, they're typically going to return data from within
> > the current memory context, but could legitimately also return part of the
> > data from curaggcontext. Perhaps we could forbid that?
>
>
Andres Freund writes:
> If there is a finalfunc, they're typically going to return data from within
> the current memory context, but could legitimately also return part of the
> data from curaggcontext. Perhaps we could forbid that?
No, I don't think we can get away with that. See int8inc()
Hi,
On 2022-09-07 11:08:27 -0400, Tom Lane wrote:
> > I'll need to think about how best to fix this. In the meantime, I
> > think the other 32-bit animals are probably not going to like this
> > either :-(
>
> Yeah. The basic problem here is that we've greatly reduced the amount
> of redundancy
On Thu, 8 Sept 2022 at 09:32, David Rowley wrote:
>
> On Thu, 8 Sept 2022 at 03:08, Tom Lane wrote:
> > Step 4 is annoyingly expensive, but perhaps not too awful given
> > the way we step up alloc block sizes. We should make sure that
> > any context we want to use MemoryContextContains with is
On Thu, 8 Sept 2022 at 03:08, Tom Lane wrote:
> 4. For aset.c, I'd be inclined to have it compute the AllocBlock
> address implied by the putative chunk header, and then run through
> the context's alloc blocks and see if any of them match. If we
> do find a match, and the chunk address is
David Rowley writes:
> Looks like my analysis wasn't that good in nodeWindowAgg.c. The
> reason it's crashing is due to int2int4_sum() returning
> Int64GetDatumFast(transdata->sum).
Ugh. I thought for a bit about whether we could define that as wrong,
but it's returning a portion of its input,
On Thu, 8 Sept 2022 at 01:22, David Rowley wrote:
>
> On Thu, 8 Sept 2022 at 01:05, Julien Rouhaud wrote:
> > FYI lapwing isn't happy with this patch:
> > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=lapwing=2022-09-07%2012%3A40%3A16.
>
> I'll look into it further.
Looks like my
On Thu, 8 Sept 2022 at 01:05, Julien Rouhaud wrote:
> FYI lapwing isn't happy with this patch:
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=lapwing=2022-09-07%2012%3A40%3A16.
Thanks. It does seem to be because of the nodeWindowAgg.c usage of
MemoryContextContains.
I'll look into it
Hi,
On Thu, Sep 08, 2022 at 12:29:22AM +1200, David Rowley wrote:
>
> I spent some time looking at our existing usages of
> MemoryContextContains() to satisfy myself that we'll only ever pass in
> a pointer to memory allocated by a MemoryContext and pushed this
> patch.
FYI lapwing isn't happy
On Tue, 6 Sept 2022 at 15:17, David Rowley wrote:
>
> On Tue, 6 Sept 2022 at 14:43, Tom Lane wrote:
> > I think MemoryContextContains' charter is to return
> >
> > GetMemoryChunkContext(pointer) == context
> >
> > *except* that instead of asserting what GetMemoryChunkContext asserts,
> >
В Ср, 07/09/2022 в 16:13 +1200, David Rowley пишет:
> On Tue, 6 Sept 2022 at 01:41, David Rowley wrote:
> >
> > On Fri, 2 Sept 2022 at 20:11, David Rowley wrote:
> > >
> > > On Thu, 1 Sept 2022 at 12:46, Tom Lane wrote:
> > > >
> > > > David Rowley writes:
> > > > > Maybe we should just
David Rowley writes:
> It seems a pretty large portion of allocation request sizes are
> power-of-2 sized and use AllocSet.
No surprise there, we've been programming with aset.c's behavior
in mind for ~20 years ...
regards, tom lane
On Tue, 6 Sept 2022 at 01:41, David Rowley wrote:
>
> On Fri, 2 Sept 2022 at 20:11, David Rowley wrote:
> >
> > On Thu, 1 Sept 2022 at 12:46, Tom Lane wrote:
> > >
> > > David Rowley writes:
> > > > Maybe we should just consider always making room for a sentinel for
> > > > chunks that are on
On Tue, 6 Sept 2022 at 14:43, Tom Lane wrote:
> I think MemoryContextContains' charter is to return
>
> GetMemoryChunkContext(pointer) == context
>
> *except* that instead of asserting what GetMemoryChunkContext asserts,
> it should treat those cases as reasons to return false. So if you
David Rowley writes:
> I think the fix is harder than I thought, or perhaps impossible to do
> given how we now determine the owning MemoryContext of a pointer.
> There's a comment in MemoryContextContains which says:
> * NB: Can't use GetMemoryChunkContext() here - that performs assertions
> *
On Tue, 6 Sept 2022 at 14:32, David Rowley wrote:
> I wonder if there are many usages of MemoryContextContains in
> extensions. If there's not, I'd be much happier if we got rid of this
> function and used GetMemoryChunkContext() in nodeAgg.c and
> nodeWindowAgg.c.
I see postgis is one user of
On Tue, 6 Sept 2022 at 12:27, Tom Lane wrote:
>
> David Rowley writes:
> > On Tue, 6 Sept 2022 at 11:09, Andres Freund wrote:
> >> I was looking at
> >> MemoryContextContains(). Unless I am missing something, the patch omitted
> >> adjusting that? We'll probably always return false right now.
>
David Rowley writes:
> On Tue, 6 Sept 2022 at 11:09, Andres Freund wrote:
>> I was looking at
>> MemoryContextContains(). Unless I am missing something, the patch omitted
>> adjusting that? We'll probably always return false right now.
> Oops. Yes. I'll push a fix a bit later.
The existing
On Tue, 6 Sept 2022 at 11:09, Andres Freund wrote:
> I was looking at
> MemoryContextContains(). Unless I am missing something, the patch omitted
> adjusting that? We'll probably always return false right now.
Oops. Yes. I'll push a fix a bit later.
David
Hi,
On 2022-08-29 17:26:29 +1200, David Rowley wrote:
> On Mon, 29 Aug 2022 at 10:39, David Rowley wrote:
> > One more try to make CFbot happy.
>
> After a bit more revision, mostly updating outdated comments and
> naming adjustments, I've pushed this.
Responding to Tom's email about guc.c
On Fri, 2 Sept 2022 at 20:11, David Rowley wrote:
>
> On Thu, 1 Sept 2022 at 12:46, Tom Lane wrote:
> >
> > David Rowley writes:
> > > Maybe we should just consider always making room for a sentinel for
> > > chunks that are on dedicated blocks. At most that's an extra 8 bytes
> > > in some
On Thu, 1 Sept 2022 at 12:46, Tom Lane wrote:
>
> David Rowley writes:
> > Maybe we should just consider always making room for a sentinel for
> > chunks that are on dedicated blocks. At most that's an extra 8 bytes
> > in some allocation that's either over 1024 or 8192 (depending on
> >
Hi,
Excuse me for posting on this thread.
Coverity has a complaints about aset.c
CID 1497225 (#1 of 2): Out-of-bounds write (OVERRUN)3. overrun-local:
Overrunning
array set->freelist of 11 8-byte elements at element index 1073741823 (byte
offset 8589934591) using index fidx (which evaluates to
David Rowley writes:
> On Thu, 1 Sept 2022 at 16:06, Tom Lane wrote:
>> It doesn't seem to be fixing any live bug in the back branches,
>> but by the same token it's harmless.
> I considered that an extension might use the Slab allocator with a
> non-MAXALIGNED chunksize and might run into some
On Thu, 1 Sept 2022 at 16:06, Tom Lane wrote:
>
> David Rowley writes:
> > Does anyone have any objections to d5ee4db0e in its entirety being
> > backpatched?
>
> It doesn't seem to be fixing any live bug in the back branches,
> but by the same token it's harmless.
I considered that an
David Rowley writes:
> Does anyone have any objections to d5ee4db0e in its entirety being
> backpatched?
It doesn't seem to be fixing any live bug in the back branches,
but by the same token it's harmless.
regards, tom lane
On Tue, 30 Aug 2022 at 13:16, David Rowley wrote:
> I'm also wondering if this should also be backpatched back to v10,
> providing the build farm likes it well enough on master.
Does anyone have any objections to d5ee4db0e in its entirety being backpatched?
David
Tomas Vondra writes:
> You're probably right we'll notice the clobber cases due to corruption
> of the next chunk header. The annoying thing is having a corrupted
> header only tells you there's a corruption somewhere, but it may be hard
> to know which part of the code caused it.
Same's true of
On 9/1/22 02:23, Tom Lane wrote:
> Tomas Vondra writes:
>> Focusing on the aset, vast majority of allocations (60M out of 64M) is
>> small enough to use power-of-2 logic, and we go from 6.3GB to 8.2GB, so
>> ~30%. Not great, not terrible.
>
> Not sure why this escaped me before, but I
David Rowley writes:
> Maybe we should just consider always making room for a sentinel for
> chunks that are on dedicated blocks. At most that's an extra 8 bytes
> in some allocation that's either over 1024 or 8192 (depending on
> maxBlockSize).
Agreed, if we're not doing that already then we
On Thu, 1 Sept 2022 at 12:23, Tom Lane wrote:
> Is there reason to think we can't validate headers enough to catch
> clobbers?
For non-sentinel chunks, the next byte after the end of the chunk will
be storing the block offset for the following chunk. I think:
if (block !=
Tomas Vondra writes:
> Focusing on the aset, vast majority of allocations (60M out of 64M) is
> small enough to use power-of-2 logic, and we go from 6.3GB to 8.2GB, so
> ~30%. Not great, not terrible.
Not sure why this escaped me before, but I remembered another argument
for not forcibly adding
On 8/31/22 23:46, David Rowley wrote:
> On Thu, 1 Sept 2022 at 08:53, Tomas Vondra
> wrote:
>> So the raw size (what we asked for) is ~23.5GB, but in practice we
>> allocate ~28.8GB because of the pow-of-2 logic. And by adding the extra
>> 1B we end up allocating 31.5GB. That doesn't seem like
On Thu, 1 Sept 2022 at 08:53, Tomas Vondra
wrote:
> So the raw size (what we asked for) is ~23.5GB, but in practice we
> allocate ~28.8GB because of the pow-of-2 logic. And by adding the extra
> 1B we end up allocating 31.5GB. That doesn't seem like a huge increase,
> and it's far from the +60%
On 8/31/22 00:40, David Rowley wrote:
> On Wed, 31 Aug 2022 at 02:17, Tom Lane wrote:
>>
>> I wrote:
>>> So maybe we should revisit the question. It'd be worth collecting some
>>> stats about how much extra space would be needed if we force there
>>> to be room for a sentinel.
>>
>> Actually,
On Wed, 31 Aug 2022 at 02:17, Tom Lane wrote:
>
> I wrote:
> > So maybe we should revisit the question. It'd be worth collecting some
> > stats about how much extra space would be needed if we force there
> > to be room for a sentinel.
>
> Actually, after ingesting more caffeine, the problem
David Rowley writes:
> Here's a patch which adds a comment to MemoryContextMethodID to Robert's
> patch.
OK, but while looking at that I noticed the adjacent
#define MEMORY_CONTEXT_METHODID_MASK \
UINT64CONST((1 << MEMORY_CONTEXT_METHODID_BITS) - 1)
I'm rather astonished that that
On Tue, Aug 30, 2022 at 11:39 AM David Rowley wrote:
> On Wed, 31 Aug 2022 at 03:31, David Rowley wrote:
> > I've no objections to adding a comment to the enum to
> > explain to future devs. My vote would be for that and add the (int)
> > cast as proposed by Robert.
>
> Here's a patch which adds
On Wed, 31 Aug 2022 at 03:31, David Rowley wrote:
> I've no objections to adding a comment to the enum to
> explain to future devs. My vote would be for that and add the (int)
> cast as proposed by Robert.
Here's a patch which adds a comment to MemoryContextMethodID to Robert's patch.
David
On Wed, 31 Aug 2022 at 01:36, Tom Lane wrote:
>
> David Rowley writes:
> > I think the Assert is useful as if we were ever to add an enum member
> > with the value of 8 and forgot to adjust MEMORY_CONTEXT_METHODID_BITS
> > then bad things would happen inside MemoryChunkSetHdrMask() and
> >
On Wed, 31 Aug 2022 at 03:00, Robert Haas wrote:
>
> On Tue, Aug 30, 2022 at 3:14 AM David Rowley wrote:
> > I think the Assert is useful as if we were ever to add an enum member
> > with the value of 8 and forgot to adjust MEMORY_CONTEXT_METHODID_BITS
> > then bad things would happen inside
On Tue, Aug 30, 2022 at 3:14 AM David Rowley wrote:
> I'm not really sure either. I tried compiling with clang 12.0.1 with
> -Wtautological-constant-out-of-range-compare and don't get this
> warning.
I have a much older clang version, it seems. clang -v reports 5.0.2. I
use -Wall and -Werror as
I wrote:
> So maybe we should revisit the question. It'd be worth collecting some
> stats about how much extra space would be needed if we force there
> to be room for a sentinel.
Actually, after ingesting more caffeine, the problem with this for aset.c
is that the only way to add space for a
David Rowley writes:
> On Tue, 30 Aug 2022 at 03:16, Robert Haas wrote:
>> ../../../../src/include/utils/memutils_memorychunk.h:186:18: error:
>> comparison of constant 7 with expression of type
>> 'MemoryContextMethodID' (aka 'enum MemoryContextMethodID') is always
>> true
Tomas Vondra writes:
> I guess the idea was to add a sentinel only when there already is space
> for it, but perhaps that's a bad tradeoff limiting the benefits. Either
> we add the sentinel fairly often (and then why not just add it all the
> time - it'll need a bit more space), or we do it only
On 8/30/22 04:31, David Rowley wrote:
> On Tue, 30 Aug 2022 at 13:55, Tom Lane wrote:
>> I wonder if slab ought to artificially bump up such requests when
>> MEMORY_CONTEXT_CHECKING is enabled, so there's room for a sentinel.
>> I think it's okay for aset.c to not do that, because its
On 8/30/22 03:04, David Rowley wrote:
> On Tue, 30 Aug 2022 at 12:22, Tomas Vondra
> wrote:
>> I also suggested doing a similar check in MemoryChunkGetPointer, so that
>> we catch the issue earlier - right after we allocate the chunk. Any
>> opinion on that?
>
> I think it's probably a good
On Tue, 30 Aug 2022 at 03:16, Robert Haas wrote:
> ../../../../src/include/utils/memutils_memorychunk.h:186:18: error:
> comparison of constant 7 with expression of type
> 'MemoryContextMethodID' (aka 'enum MemoryContextMethodID') is always
> true
David Rowley writes:
> I can revert df0f4feef, but would prefer just to get the green light
> for d5ee4db0e from those 32-bit arm animals before doing so.
I have a check-world pass on my RPI3 (Fedora 30 armv7l image).
PPC test still running, but I don't doubt it will pass; it's
finished
On Tue, 30 Aug 2022 at 15:15, Tom Lane wrote:
> AFAICS that could only happen if "double" has 8-byte alignment
> requirement but int64 does not. I recall some discussion about
> that possibility a month or two back, but I think we concluded
> that we weren't going to support it.
ok
> I guess
David Rowley writes:
> On Tue, 30 Aug 2022 at 03:39, Tom Lane wrote:
>> I'd suggest reverting df0f4feef as it seems to be
>> a red herring.
> I think it's useless providing that a 64-bit variable will always be
> aligned to 8 bytes on all of our supported 32-bit platforms as,
> without the
On Tue, 30 Aug 2022 at 03:39, Tom Lane wrote:
> I'd suggest reverting df0f4feef as it seems to be
> a red herring.
I think it's useless providing that a 64-bit variable will always be
aligned to 8 bytes on all of our supported 32-bit platforms as,
without the padding, the uint64 hdrmask in
On Tue, 30 Aug 2022 at 13:58, Tomas Vondra
wrote:
>
> On 8/30/22 03:16, David Rowley wrote:
> > Any chance you could run make check-world on your 32-bit Raspberry PI?
> >
>
> Will do, but I think the sentinel fix should be
Thank you. I think Tom is also running make check-world. I now have
an
On Tue, 30 Aug 2022 at 13:55, Tom Lane wrote:
> I wonder if slab ought to artificially bump up such requests when
> MEMORY_CONTEXT_CHECKING is enabled, so there's room for a sentinel.
> I think it's okay for aset.c to not do that, because its power-of-2
> behavior means there usually is room for
On Tue, 30 Aug 2022 at 13:58, Tomas Vondra
wrote:
> armv7l (32-bit rpi4)
>
> +WARNING: chunkSize 216 fullChunkSize 232 header 16
> +WARNING: chunkSize 64 fullChunkSize 80 header 16
>
> aarch64 (64-bit rpi4)
>
> +WARNING: chunkSize 304 fullChunkSize 320 header 16
> +WARNING: chunkSize 80
On 8/30/22 03:55, Tom Lane wrote:
> Tomas Vondra writes:
>> On 8/30/22 02:45, David Rowley wrote:
>>> I think the existing sentinel check looks wrong:
>>> if (!sentinel_ok(chunk, slab->chunkSize))
>>> shouldn't that be passing the pointer rather than the chunk?
>
>> I agree the check in
On 8/30/22 03:16, David Rowley wrote:
> On Tue, 30 Aug 2022 at 12:45, David Rowley wrote:
>> I think the existing sentinel check looks wrong:
>>
>> if (!sentinel_ok(chunk, slab->chunkSize))
>>
>> shouldn't that be passing the pointer rather than the chunk?
>
> Here's v2 of the slab-fix patch.
>
1 - 100 of 149 matches
Mail list logo
