On Tue, Apr 16, 2024 at 3:44 PM David Rowley wrote:
> On Tue, 16 Apr 2024 at 17:13, Amul Sul wrote:
> > Attached is a small patch adding the missing BumpContext description to
> the
> > README.
>
> Thanks for noticing and working on the patch.
>
> There were a few things that were not quite
On Mon, 8 Apr 2024 at 00:37, David Rowley wrote:
> I've now pushed all 3 patches. Thank you for all the reviews on
> these and for the extra MemoryContextMethodID bit, Matthias.
I realised earlier today when working on [1] that bump makes a pretty
brain-dead move when adding dedicated blocks
On Tue, 16 Apr 2024 at 17:13, Amul Sul wrote:
> Attached is a small patch adding the missing BumpContext description to the
> README.
Thanks for noticing and working on the patch.
There were a few things that were not quite accurate or are misleading:
1.
> +These three memory contexts aim to
> On 16 Apr 2024, at 07:12, Amul Sul wrote:
>
> Attached is a small patch adding the missing BumpContext description to the
> README.
Nice catch, we should add it to the README.
+ pfree'd or realloc'd.
I think it's best to avoid mixing API:s, "pfree'd or repalloc'd" keeps it to
functions in
Attached is a small patch adding the missing BumpContext description to the
README.
Regards,
Amul
0001-Add-BumpContext-description-to-mmgr-README.patch
Description: Binary data
> On 8 Apr 2024, at 01:04, Andres Freund wrote:
> What makes you think that's unrelated? To me that looks like the same issue?
Nvm, I misread the assert, ETOOLITTLESLEEP. Sorry for the noise.
--
Daniel Gustafsson
On Mon, 8 Apr 2024 at 09:09, Andres Freund wrote:
> I suspect that KeeperBlock() isn't returning true, because IsKeeperBlock
> misses
> the MAXALIGN(). I think that about fits with:
Thanks for investigating that.
I've just pushed a fix for the macro and also adjusted a location
which was
Hi,
On 2024-04-08 00:55:42 +0200, Daniel Gustafsson wrote:
> > On 8 Apr 2024, at 00:41, Tom Lane wrote:
> >
> > Tomas Vondra writes:
> >> Yup, changing it to this:
> >
> >> #define KeeperBlock(set) ((BumpBlock *) ((char *) (set) +
> >> MAXALIGN(sizeof(BumpContext
> >
> >> fixes the issue
> On 8 Apr 2024, at 00:41, Tom Lane wrote:
>
> Tomas Vondra writes:
>> Yup, changing it to this:
>
>> #define KeeperBlock(set) ((BumpBlock *) ((char *) (set) +
>> MAXALIGN(sizeof(BumpContext
>
>> fixes the issue for me.
>
> Mamba is happy with that change, too.
Unrelated to that one,
Tomas Vondra writes:
> Yup, changing it to this:
> #define KeeperBlock(set) ((BumpBlock *) ((char *) (set) +
> MAXALIGN(sizeof(BumpContext
> fixes the issue for me.
Mamba is happy with that change, too.
regards, tom lane
On 4/7/24 23:09, Andres Freund wrote:
> Hi,
>
> On 2024-04-07 22:35:47 +0200, Tomas Vondra wrote:
>> I haven't investigated, but I'd considering it works on 64-bit, I guess
>> it's not considering alignment somewhere. I can dig more if needed.
>
> I think I may the problem:
>
>
> #define
Hi,
On 2024-04-07 22:35:47 +0200, Tomas Vondra wrote:
> I haven't investigated, but I'd considering it works on 64-bit, I guess
> it's not considering alignment somewhere. I can dig more if needed.
I think I may the problem:
#define KeeperBlock(set) ((BumpBlock *) ((char *) (set) +
On 4/7/24 22:35, Tomas Vondra wrote:
> On 4/7/24 14:37, David Rowley wrote:
>> On Sun, 7 Apr 2024 at 22:05, John Naylor wrote:
>>>
>>> On Sat, Apr 6, 2024 at 7:37 PM David Rowley wrote:
>>> I'm planning on pushing these, pending a final look at 0002 and 0003
on Sunday morning NZ
On 4/7/24 14:37, David Rowley wrote:
> On Sun, 7 Apr 2024 at 22:05, John Naylor wrote:
>>
>> On Sat, Apr 6, 2024 at 7:37 PM David Rowley wrote:
>>>
>> I'm planning on pushing these, pending a final look at 0002 and 0003
>>> on Sunday morning NZ time (UTC+12), likely in about 10 hours time.
>>
On Sun, 7 Apr 2024 at 22:05, John Naylor wrote:
>
> On Sat, Apr 6, 2024 at 7:37 PM David Rowley wrote:
> >
> I'm planning on pushing these, pending a final look at 0002 and 0003
> > on Sunday morning NZ time (UTC+12), likely in about 10 hours time.
>
> +1
I've now pushed all 3 patches. Thank
On Sat, Apr 6, 2024 at 7:37 PM David Rowley wrote:
>
I'm planning on pushing these, pending a final look at 0002 and 0003
> on Sunday morning NZ time (UTC+12), likely in about 10 hours time.
+1
I haven't looked at v6, but I've tried using it in situ, and it seems
to work as well as hoped:
On Sun, 7 Apr 2024, 01:59 David Rowley, wrote:
> On Sun, 7 Apr 2024 at 05:45, Matthias van de Meent
> wrote:
> > Malloc's docs specify the minimum chunk size at 4*sizeof(void*) and
> itself uses , so using powers of 2 for chunks would indeed fail to detect
> 1s in the 4th bit. I suspect you'll
On Sun, 7 Apr 2024 at 05:45, Matthias van de Meent
wrote:
> Malloc's docs specify the minimum chunk size at 4*sizeof(void*) and itself
> uses , so using powers of 2 for chunks would indeed fail to detect 1s in the
> 4th bit. I suspect you'll get different results when you check the allocation
On Sat, 6 Apr 2024, 14:36 David Rowley, wrote:
> On Sat, 6 Apr 2024 at 02:30, Matthias van de Meent
> wrote:
> >
> > On Thu, 4 Apr 2024 at 22:43, Tom Lane wrote:
> > >
> > > Matthias van de Meent writes:
> > > > It extends memory context IDs to 5 bits (32 values), of which
> > > > - 8 have
On Sat, 6 Apr 2024 at 03:24, Tom Lane wrote:
> OK. I did not read the patch very closely, but at least in principle
> I have no further objections. David, are you planning to take point
> on getting this in?
Yes. I'll be looking soon.
David
Matthias van de Meent writes:
> On Thu, 4 Apr 2024 at 22:43, Tom Lane wrote:
>> The only objection I can think of is that perhaps this would slow
>> things down a tad by requiring more complicated shifting/masking.
>> I wonder if we could redo the performance checks that were done
>> on the way
On Thu, 4 Apr 2024 at 22:43, Tom Lane wrote:
>
> Matthias van de Meent writes:
> > On Mon, 25 Mar 2024 at 22:44, Tom Lane wrote:
> >> Basically, I'm not happy with consuming the last reasonably-available
> >> pattern for a memory context type that has little claim to being the
> >> Last Context
Matthias van de Meent writes:
> On Mon, 25 Mar 2024 at 22:44, Tom Lane wrote:
>> Basically, I'm not happy with consuming the last reasonably-available
>> pattern for a memory context type that has little claim to being the
>> Last Context Type We Will Ever Want. Rather than making a further
>>
On Mon, 25 Mar 2024 at 22:44, Tom Lane wrote:
>
> David Rowley writes:
> > On Tue, 26 Mar 2024 at 03:53, Tom Lane wrote:
> >> Could we move the knowledge of exactly which context type it is out
> >> of the per-chunk header and keep it in the block header?
>
> > I wasn't 100% clear on your
David Rowley writes:
> On Tue, 26 Mar 2024 at 03:53, Tom Lane wrote:
>> Could we move the knowledge of exactly which context type it is out
>> of the per-chunk header and keep it in the block header?
> I wasn't 100% clear on your opinion about using 010 vs expanding the
> bit-space. Based on
On Tue, 26 Mar 2024 at 03:53, Tom Lane wrote:
> I agree with this completely. However, the current design for chunk
> headers is mighty restrictive about how many kinds of contexts we can
> have. We need to open that back up.
Andres mentioned how we could do this in [1]. One possible issue
Tomas Vondra writes:
> IMHO the idea of having a general purpose memory context and then also
> specialized memory contexts for particular allocation patterns is great,
> and we should embrace it. Adding more and more special cases into
> AllocSet seems to go directly against that idea, makes the
On 3/25/24 12:41, David Rowley wrote:
> On Tue, 5 Mar 2024 at 15:42, David Rowley wrote:
>> The query I ran was:
>>
>> select chksz,mtype,pg_allocate_memory_test_reset(chksz,
>> 1024*1024,1024*1024*1024, mtype) from (values(8),(16),(32),(64))
>>
On Tue, 5 Mar 2024 at 15:42, David Rowley wrote:
> The query I ran was:
>
> select chksz,mtype,pg_allocate_memory_test_reset(chksz,
> 1024*1024,1024*1024*1024, mtype) from (values(8),(16),(32),(64))
> sizes(chksz),(values('aset'),('generation'),('slab'),('bump'))
> cxt(mtype) order by
On 3/15/24 03:21, David Rowley wrote:
> On Tue, 12 Mar 2024 at 23:57, Tomas Vondra
> wrote:
>> Attached is an updated version of the mempool patch, modifying all the
>> memory contexts (not just AllocSet), including the bump context. And
>> then also PDF with results from the two machines,
On Tue, 12 Mar 2024 at 23:57, Tomas Vondra
wrote:
> Attached is an updated version of the mempool patch, modifying all the
> memory contexts (not just AllocSet), including the bump context. And
> then also PDF with results from the two machines, comparing results
> without and with the mempool.
On Tue, Mar 12, 2024 at 6:41 AM David Rowley wrote:
> Thanks for trying this out. I didn't check if the performance was
> susceptible to the memory size before the reset. It certainly would
> be once the allocation crosses some critical threshold of CPU cache
> size, but probably it will also
On Mon, 11 Mar 2024 at 22:09, John Naylor wrote:
> I ran the test function, but using 256kB and 3MB for the reset
> frequency, and with 8,16,24,32 byte sizes (patched against a commit
> after the recent hot/cold path separation). Images attached. I also
> get a decent speedup with the bump
On Tue, 12 Mar 2024 at 12:25, Tomas Vondra
wrote:
> (b) slab is considerably slower
It would be interesting to modify SlabReset() to, instead of free()ing
the blocks, push the first SLAB_MAXIMUM_EMPTY_BLOCKS of them onto the
emptyblocks list.
That might give us an idea of how much overhead
On Tue, Mar 5, 2024 at 9:42 AM David Rowley wrote:
> performance against the recently optimised aset, generation and slab
> contexts. The attached graph shows the time it took in seconds to
> allocate 1GB of memory performing a context reset after 1MB. The
> function I ran the test on is in the
On Wed, 21 Feb 2024 at 00:02, Matthias van de Meent
wrote:
>
> On Tue, 20 Feb 2024 at 11:02, David Rowley wrote:
> > On Fri, 26 Jan 2024 at 01:29, Matthias van de Meent
> > wrote:
> > > >> +allocSize = MAXALIGN(sizeof(BumpContext)) + Bump_BLOCKHDRSZ +
> > > >> +if (minContextSize != 0)
On Tue, 20 Feb 2024 at 23:52, Matthias van de Meent
wrote:
> What I meant was that
>
> > (char *) block + Bump_BLOCKHDRSZ
> vs
> > ((char *) block) + Bump_BLOCKHDRSZ
>
> , when combined with my little experience with pointer addition and
> precedence, and a lack of compiler at the ready at that
On Tue, 20 Feb 2024 at 11:02, David Rowley wrote:
> On Fri, 26 Jan 2024 at 01:29, Matthias van de Meent
> wrote:
> > >> +allocSize = MAXALIGN(sizeof(BumpContext)) + Bump_BLOCKHDRSZ +
> > >> +if (minContextSize != 0)
> > >> +allocSize = Max(allocSize, minContextSize);
> > >> +
On Tue, 20 Feb 2024 at 10:41, David Rowley wrote:
> On Tue, 7 Nov 2023 at 07:55, Matthias van de Meent
> wrote:
> > > +++ b/src/backend/utils/mmgr/bump.c
> > > +BumpBlockIsEmpty(BumpBlock *block)
> > > +{
> > > +/* it's empty if the freeptr has not moved */
> > > +return (block->freeptr
Thanks for taking an interest in this.
On Sat, 17 Feb 2024 at 11:46, Tomas Vondra
wrote:
> I wasn't paying much attention to these memcontext reworks in 2022, so
> my instinct was simply to use one of those "UNUSED" IDs. But after
> looking at the 80ef92675823 a bit more, are those IDs really
On Fri, 26 Jan 2024 at 01:29, Matthias van de Meent
wrote:
> >> +allocSize = MAXALIGN(sizeof(BumpContext)) + Bump_BLOCKHDRSZ +
> >> +if (minContextSize != 0)
> >> +allocSize = Max(allocSize, minContextSize);
> >> +else
> >> +allocSize = Max(allocSize, initBlockSize);
>
On Wed, 26 Jul 2023 at 12:11, Nathan Bossart wrote:
> I think it'd be okay to steal those bits. AFAICT it'd complicate the
> macros in memutils_memorychunk.h a bit, but that doesn't seem like such a
> terrible price to pay to allow us to keep avoiding the glibc bit patterns.
I've not adjusted
Thanks for having a look at this.
On Tue, 7 Nov 2023 at 07:55, Matthias van de Meent
wrote:
> I think it would make sense to split the "add a bump allocator"
> changes from the "use the bump allocator in tuplesort" patches.
I've done this and will post updated patches after replying to the
Hi,
On 2024-02-16 20:10:48 -0500, Tom Lane wrote:
> Tomas Vondra writes:
> > On 2/17/24 00:14, Tom Lane wrote:
> >> The conclusion was that the specific invalid values didn't matter as
> >> much on the other platforms as they do with glibc. But right now you
> >> have a fifty-fifty chance that
Tomas Vondra writes:
> On 2/17/24 00:14, Tom Lane wrote:
>> The conclusion was that the specific invalid values didn't matter as
>> much on the other platforms as they do with glibc. But right now you
>> have a fifty-fifty chance that a pointer to garbage will look valid.
>> Do we want to
On 2/17/24 00:14, Tom Lane wrote:
> Tomas Vondra writes:
>> Maybe I'm completely misunderstanding the implication of those limits,
>> but doesn't this mean the claim that we can support 8 memory context
>> types is not quite true, and the limit is 4, because the 4 IDs are
>> already used for
Tomas Vondra writes:
> Maybe I'm completely misunderstanding the implication of those limits,
> but doesn't this mean the claim that we can support 8 memory context
> types is not quite true, and the limit is 4, because the 4 IDs are
> already used for malloc stuff?
Well, correct code would
On 11/6/23 19:54, Matthias van de Meent wrote:
>
> ...
>
> Tangent: Do we have specific notes on worst-case memory usage of
> memory contexts with various allocation patterns? This new bump
> allocator seems to be quite efficient, but in a worst-case allocation
> pattern it can still waste about
Hi,
I wanted to take a look at this patch but it seems to need a rebase,
because of a seemingly trivial conflict in MemoryContextMethodID:
--- src/include/utils/memutils_internal.h
+++ src/include/utils/memutils_internal.h
@@ -123,12 +140,13 @@ typedef enum MemoryContextMethodID
{
On Mon, 6 Nov 2023 at 19:54, Matthias van de Meent
wrote:
>
> On Tue, 11 Jul 2023 at 01:51, David Rowley wrote:
>>
>> On Tue, 27 Jun 2023 at 21:19, David Rowley wrote:
>>> I've attached the bump allocator patch and also the script I used to
>>> gather the performance results in the first 2 tabs
On Tue, 11 Jul 2023 at 01:51, David Rowley wrote:
>
> On Tue, 27 Jun 2023 at 21:19, David Rowley wrote:
> > I've attached the bump allocator patch and also the script I used to
> > gather the performance results in the first 2 tabs in the attached
> > spreadsheet.
>
> I've attached a v2 patch
On Tue, Jun 27, 2023 at 09:19:26PM +1200, David Rowley wrote:
> Because of all of what is mentioned above about the current state of
> tuplesort, there does not really seem to be much need to have chunk
> headers in memory we allocate for tuples at all. Not having these
> saves us a further 8
On Tue, 27 Jun 2023 at 21:19, David Rowley wrote:
> I've attached the bump allocator patch and also the script I used to
> gather the performance results in the first 2 tabs in the attached
> spreadsheet.
I've attached a v2 patch which changes the BumpContext a little to
remove some of the
53 matches
Mail list logo
