On 10/25/13 7:23 AM, Manu wrote:
My immediate reactions:
1. I'm still sad there are no allocAligned() functions or something of
that type to request explicit alignment with allocations. I'm not sure
there is sufficient support for requesting alignment with allocations.
The set-able alignment property approach seems a little weird (and only
seemed to be supported on one allocator?). I guess experience will tell
if this is sufficient and/or convenient.
I'd still like to see an allocWithAlignment() method or something, which
may be implemented efficiently by allocators that can support it.
Per-allocation alignment requests are currently allowed (by setting the
property transitorily) but indeed not really nice. I can see how
HeapBlock could implement a nice alignedAllocate call but the others
would kind of boringly pass it along.
Don't forget that it's always possible to define extra primitives for a
given allocator (see e.g. relinquish or available). They should be
migrated to official API status only if they could help composition in
one way or another. In fact that's how I defined the API - I started
with allocate()/deallocate() and a list of allocators I wanted to
implement, and tried to get away with as few primitives as possible. For
example FallbackAllocator makes owns() necessary etc.
2. I see some lines like this:
assert(parent.alignment>=X.alignof);
What if parent.alignment < X.alignof? If 'parent' is something with an
inflexible alignment, like malloc or the GC, what is the proper
(convenient) way to reconcile the requirement?
That assert is in Freelist and requires that the parent allocator
returns memory aligned to at least pointer alignment, so as to write
pointers at the front of the allocation. That's a really low bar, I
think there's no need to worry about it. (If you do, defining a
UnalignedFreelist is always an option.)
3. FreeList has some options; minSize, maxSize, maxNodes. When I'm using
a freelist, the most important option to me is to be able to allocate
new nodes in batches. I'd like an option added to control the batch
size, so multiple new nodes are allocated in contiguous blocks when the
pool grows. Perhaps add a 4th parameter; minBatchSize = 1? (to retain
support for your existing logic batching small allocations into larger
allocated blocks)
The main reasons for this are cache/spatial locality of small
allocations, and minimising overhead burden on the upstream allocator.
Good idea; it's what people often do anyway. Before sending this out I'd
added at least the option to allocate several nodes at a time to fill
memory more efficiently, see allocateFresh at
https://github.com/andralex/phobos/blob/allocator/std/allocator.d#L1988.
Allowing the user to choose the number of nodes in a batch is a good
extension of that. (I think the default should be unbounded, i.e. let
the freelist allocator decide depending on goodMallocSize of the parent.)
4. OT: Working with D struct's, you often encounter, for instance:
structRegion(uintminAlign=platformAlignment)
{
privateBasicRegion!(minAlign)base;
...
Doesn't this make you sad? It makes me sad quite regularly.
Especially when the very next line is (often): alias base this;
I'm content with that and I think you should too.
Andrei
