On Friday, 12 June 2015 at 19:19:55 UTC, Andrei Alexandrescu wrote:
> On 6/12/15 11:23 AM, Baz wrote:
>> On Friday, 12 June 2015 at 11:09:01 UTC, Dicebot wrote:
>>> Small tip for reviewers: there are quite many modules in proposed
>>> package but majority is actual allocator implementation. I'd suggest
>>> to start investigating sources/documentation starting from
>>> http://erdani.com/d/phobos-prerelease/std_experimental_allocator.html
>>> and
>>> http://erdani.com/d/phobos-prerelease/std_experimental_allocator_building_blocks.html
>>> while using
>>> http://erdani.com/d/phobos-prerelease/std_experimental_allocator_showcase.html
>>> as reference for "highlight" examples.
>>
>> while building the package as a static lic library, i've found that
>> _showcase.d_ is not usable under Windows because MmapAllocator is Posix
>> only:
>>
>> https://github.com/andralex/phobos/blob/allocator/std/experimental/allocator/showcase.d#L66
>>
>> https://github.com/andralex/phobos/blob/allocator/std/experimental/allocator/mmap_allocator.d#L14
>
> I've asked this a while ago as well - could anyone with a Windows rig contribute a MmapAllocator for Windows? Thanks! -- Andrei

I have done it (https://gist.github.com/BBasile/e382be91dcc18e4d2358),
but if i run the tests of the showcase i get:

---
...\region.d(84): Error: shared method std.experimental.allocator.mmap_allocator.MmapAllocator.allocate is not callable using a non-shared object
...\showcase.d(73): Error: template instance std.experimental.allocator.region.Region!(MmapAllocator, 8u, cast(Flag)false) error instantiating
...\allocator_list.d(76): Error: struct Factory does not overload ()
...\showcase.d(79): Error: template instance std.experimental.allocator.allocator_list.AllocatorList!(Factory, NullAllocator) error instantiating
...\region.d(84,29): Error: shared method std.experimental.allocator.mmap_allocator.MmapAllocator.allocate is not callable using a non-shared object
...\showcase.d(73,20): Error: template instance std.experimental.allocator.region.Region!(MmapAllocator, 8u, cast(Flag)false) error instantiating
...\allocator_list.d(76,42): Error: struct Factory does not overload ()
...\showcase.d(79,12): Error: template instance std.experimental.allocator.allocator_list.AllocatorList!(Factory, NullAllocator) error instantiating
C:\...\allocator\showcase.d has not been compiled
---

Should i continue and propose a PR ?

On Friday, 12 June 2015 at 15:54:27 UTC, Andrei Alexandrescu wrote:
> On 6/12/15 8:54 AM, Andrei Alexandrescu wrote:
>> * A dynamicaly-typed layer that allows the user to swap allocators at
>> runtime. This is embodied in IAllocator, CAllocatorImpl, theAllocator,
>> and processAllocator.
>
> Forgot to mention - the entry point for this layer is also http://erdani.com/d/phobos-prerelease/std_experimental_allocator.html.
>
> Andrei


Are there any plans to have new and delete call out to theAllocator or processAllocator?

On 6/12/15 4:07 PM, Tofu Ninja wrote:
> Are there any plans to have new and delete call out to theAllocator or
> processAllocator?

In the future we may rig things that way. -- Andrei

Andrei, have you considered creating additional std.allocator.impl package and moving actual allocators there? Or, probably, the other way around with std.allocator.core

Existing flat hierarchy does not hint about internal structure in any way.

On 6/13/15 3:14 AM, Dicebot wrote:
> Andrei, have you considered creating additional std.allocator.impl
> package and moving actual allocators there? Or, probably, the other way
> around with std.allocator.core
>
> Existing flat hierarchy does not hint about internal structure in any way.

It's good documentation, not directories, that helps understanding internal structure. There are 23 files in std/experimental/allocator, which seems manageable. I think we're good as we are.

Andrei

On Saturday, 13 June 2015 at 15:48:31 UTC, Andrei Alexandrescu wrote:
> On 6/13/15 3:14 AM, Dicebot wrote:
>> Andrei, have you considered creating additional std.allocator.impl
>> package and moving actual allocators there? Or, probably, the other way
>> around with std.allocator.core
>>
>> Existing flat hierarchy does not hint about internal structure in any way.
>
> It's good documentation, not directories, that helps understanding internal structure. There are 23 files in std/experimental/allocator, which seems manageable. I think we're good as we are.
>
> Andrei

I haven't seen overview in documentation either

On 6/13/15 10:24 AM, Dicebot wrote:
> On Saturday, 13 June 2015 at 15:48:31 UTC, Andrei Alexandrescu wrote:
>> On 6/13/15 3:14 AM, Dicebot wrote:
>>> Andrei, have you considered creating additional std.allocator.impl
>>> package and moving actual allocators there? Or, probably, the other way
>>> around with std.allocator.core
>>>
>>> Existing flat hierarchy does not hint about internal structure in any
>>> way.
>>
>> It's good documentation, not directories, that helps understanding
>> internal structure. There are 23 files in std/experimental/allocator,
>> which seems manageable. I think we're good as we are.
>>
>> Andrei
>
> I haven't seen overview in documentation either

Yah, that should go in package.d. It's reasonable to mark that as an acceptance requirement even for experimental. Will get into it soon. -- Andrei

On 2015-06-12 13:06, Dicebot wrote:
> The legendary allocator package by Andrei Alexandrescu has arrived at
> your doorsteps and kindly asks to let it into Phobos
>
> http://wiki.dlang.org/Review/std.experimental.allocator
>
> Docs: http://erdani.com/d/phobos-prerelease/std_experimental_allocator.html
> Code:
> https://github.com/andralex/phobos/tree/allocator/std/experimental/allocator

I think "IAllocator", "theAllocator" and "it" are really bad names. I recommend renaming those symbols to:

IAllocator -> Allocator
theAllocator -> currentAllocator or tlsAllocator
it -> allocator or instance. Even better if a static "opDispatch" could be used to forward all methods to the instance.

-- 
/Jacob Carlborg

On Saturday, 13 June 2015 at 15:48:31 UTC, Andrei Alexandrescu wrote:
> On 6/13/15 3:14 AM, Dicebot wrote:
>> Andrei, have you considered creating additional std.allocator.impl
>> package and moving actual allocators there? Or, probably, the other way
>> around with std.allocator.core
>>
>> Existing flat hierarchy does not hint about internal structure in any way.
>
> It's good documentation, not directories, that helps understanding internal structure. There are 23 files in std/experimental/allocator, which seems manageable. I think we're good as we are.
>
> Andrei

Personally, I disagree, the flat hierarchy was the first thing I noticed and I thought the same thing as Dicebot.

And its 23 now, but what about when new allocators get added. I don't think there is really a cost to moving it to a new folder, just makes things clearer. Any ways isn't the flat hierarchy something people have been complaining about Phobos in general?

On Saturday, 13 June 2015 at 15:48:31 UTC, Andrei Alexandrescu wrote:
> On 6/13/15 3:14 AM, Dicebot wrote:
>> Andrei, have you considered creating additional std.allocator.impl
>> package and moving actual allocators there? Or, probably, the other way
>> around with std.allocator.core
>>
>> Existing flat hierarchy does not hint about internal structure in any way.
>
> It's good documentation, not directories, that helps understanding internal structure. There are 23 files in std/experimental/allocator, which seems manageable. I think we're good as we are.
>
> Andrei

I also think putting some of the files in folder will make things cleaner and easier to understand.

These files:
std.experimental.allocator.affix_allocator,
std.experimental.allocator.allocator_list,
std.experimental.allocator.bucketizer,
std.experimental.allocator.fallback_allocator,
std.experimental.allocator.free_list,
std.experimental.allocator.free_tree,
std.experimental.allocator.gc_allocator,
std.experimental.allocator.bitmapped_block,
std.experimental.allocator.kernighan_ritchie,
std.experimental.allocator.mallocator,
std.experimental.allocator.mmap_allocator,
std.experimental.allocator.null_allocator,
std.experimental.allocator.quantizer,
std.experimental.allocator.region,
std.experimental.allocator.segregator,
std.experimental.allocator.stats_collector;

are great candidates for a std.experimental.allocator.building_blocks folder.