I reproduced the quadradic pfree performance problem and verified that these patches solved it.
The slab.c data structures and functions contain no quadradic components. I noticed the sizing loop in SlabContextCreate() and came up with a similar formula to determine chunksPerBlock that you arrived at. > Firstly, I've realized there's an issue when chunkSize gets too > large - once it exceeds blockSize, the SlabContextCreate() fails > as it's impossible to place a single chunk into the block. In > reorderbuffer, this may happen when the tuples (allocated in > tup_context) get larger than 8MB, as the context uses > SLAB_LARGE_BLOCK_SIZE (which is 8MB). > > But maybe there's a simpler solution - we may simply cap the > chunkSize (in GenSlab) to ALLOC_CHUNK_LIMIT. That's fine, because > AllocSet handles those requests in a special way - for example > instead of tracking them in freelist, those chunks got freed > immediately. I like this approach because it fixes the performance problems with smaller allocations and doesn't change how larger allocations are handled. In slab.c it looks like a line in the top comments could be clearer. Perhaps this is what is meant. < * (plus alignment), now wasting memory. > * (plus alignment), not wasting memory. In slab.c some lines are over 80 characters could be folded. It would be nice to give each patch version a unique file name. Nice patch, I enjoyed reading it! Best, John John Gorman On Mon, Sep 26, 2016 at 10:10 PM, Tomas Vondra <tomas.von...@2ndquadrant.com > wrote: > Hi, > > Attached is v2 of the patch, updated based on the review. That means: > > - Better comment explaining how free chunks are tracked in Slab context. > > - Removed the unused SlabPointerIsValid macro. > > - Modified the comment before SlabChunkData, explaining how it relates > to StandardChunkHeader. > > - Replaced the two Assert() calls with elog(). > > - Implemented SlabCheck(). I've ended up with quite a few checks there, > checking pointers between the context, block and chunks, checks due > to MEMORY_CONTEXT_CHECKING etc. And of course, cross-checking the > number of free chunks (bitmap, freelist vs. chunk header). > > - I've also modified SlabContextCreate() to compute chunksPerBlock a > bit more efficiently (use a simple formula instead of the loop, which > might be a bit too expensive for large blocks / small chunks). > > > I haven't done any changes to GenSlab, but I do have a few notes: > > Firstly, I've realized there's an issue when chunkSize gets too large - > once it exceeds blockSize, the SlabContextCreate() fails as it's impossible > to place a single chunk into the block. In reorderbuffer, this may happen > when the tuples (allocated in tup_context) get larger than 8MB, as the > context uses SLAB_LARGE_BLOCK_SIZE (which is 8MB). > > For Slab the elog(ERROR) is fine as both parameters are controlled by the > developer directly, but GenSlab computes the chunkSize on the fly, so we > must not let it fail like that - that'd result in unpredictable failures, > which is not very nice. > > I see two ways to fix this. We may either increase the block size > automatically - e.g. instead of specifying specifying chunkSize and > blockSize when creating the Slab, specify chunkSize and chunksPerBlock (and > then choose the smallest 2^k block large enough). For example with > chunkSize=96 and chunksPerBlock=1000, we'd get 128kB blocks, as that's the > closest 2^k block larger than 96000 bytes. > > But maybe there's a simpler solution - we may simply cap the chunkSize (in > GenSlab) to ALLOC_CHUNK_LIMIT. That's fine, because AllocSet handles those > requests in a special way - for example instead of tracking them in > freelist, those chunks got freed immediately. > > > regards > > -- > Tomas Vondra http://www.2ndQuadrant.com > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > >
