www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Phobos addition formal review: std.experimental.allocator

reply "Dicebot" <public dicebot.lv> writes:
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

This proposal will undergo the review/inclusion process similar 
to std.experimental.logger :

1. 2 week review/discussion period
2. if proposal author feels confident, 2 week voting period for 
accepting into sts.experimental
3. once enough field trial feedback has been accumulated, vote 
for moving into std mainline during the upcoming release beta 
period

If reviewing find some fundamental issues or voting will fail, 
the whole process will repeats again per proposal authors request.

Please start destruction, review period ends on 26.06 (23.59 +0 
Greenwich)
Jun 12 2015
next sibling parent reply "Dicebot" <public dicebot.lv> writes:
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_
uilding_blocks.html 
while using 
http://erdani.com/d/phobos-prerelease/std_experimental_allocator_showcase.html 
as reference for "highlight" examples.
Jun 12 2015
next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/12/15 4:08 AM, 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.
A few more words on that: Think of std.experimental.allocator as a three-layered cake: * An untyped layer, dealing exclusively in void[], and where all action is happening. That's where work is getting done, and also where assembly of various custom allocators happens. Best entry point for that is http://erdani.com/d/phobos-prerelease/std_experimental_allocator_b ilding_blocks.html, which describes this layer's design and links to all untyped components. * A statically-typed layer, which takes types from the user and gives back typed memory. There are two good entry points here: http://erdani.com/d/phobos-prerelease/std_experimental_allocator.html contains generic routines for creating and destroying typed objects using any untyped allocator: make, dispose, etc. There is also assembly at this level, i.e. deciding to use different heaps for shared vs. unshared data: http://erdani.com/d/phobos-prerelease/std_experimental_allocator_typed.html (this is particularly beautiful or awful, depending). * A dynamicaly-typed layer that allows the user to swap allocators at runtime. This is embodied in IAllocator, CAllocatorImpl, theAllocator, and processAllocator. Thanks in advance for reviewing this! Andrei
Jun 12 2015
next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
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
Jun 12 2015
parent reply "Tofu Ninja" <emmons0 purdue.edu> writes:
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?
Jun 12 2015
parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
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
Jun 12 2015
prev sibling parent reply "Dicebot" <public dicebot.lv> writes:
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.
Jun 13 2015
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
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
Jun 13 2015
next sibling parent reply "Dicebot" <public dicebot.lv> writes:
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
Jun 13 2015
parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
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
Jun 13 2015
prev sibling next sibling parent "Tofu Ninja" <emmons0 purdue.edu> writes:
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?
Jun 13 2015
prev sibling parent reply "ZombineDev" <valid_email he.re> writes:
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.
Jun 13 2015
next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/13/15 4:16 PM, ZombineDev 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 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.
So we have: * 1 request to change names; * 3 requests to wank around the directory structure; * 0 of everything else. Sigh. Andrei
Jun 13 2015
next sibling parent "weaselcat" <weaselcat gmail.com> writes:
On Sunday, 14 June 2015 at 00:24:51 UTC, Andrei Alexandrescu 
wrote:
 On 6/13/15 4:16 PM, ZombineDev 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 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.
So we have: * 1 request to change names; * 3 requests to wank around the directory structure; * 0 of everything else. Sigh. Andrei
can I make a request to just get it the hell in std.experimental for 2.068?
Jun 13 2015
prev sibling next sibling parent "HaraldZealot" <harald_zealot tut.by> writes:
 So we have:

 * 1 request to change names;
 * 3 requests to wank around the directory structure;
 * 0 of everything else.

 Sigh.


 Andrei
Yesterday I have found an error in documentation and have left a comment about it on github PR. And I only start to read the documentation, so I think there will be plenty small things to polish.
Jun 13 2015
prev sibling next sibling parent reply "Dicebot" <public dicebot.lv> writes:
On Sunday, 14 June 2015 at 00:24:51 UTC, Andrei Alexandrescu 
wrote:
 So we have:

 * 1 request to change names;
 * 3 requests to wank around the directory structure;
 * 0 of everything else.

 Sigh.
That is to be expected and intended for formal Phobos review. Implementation is not of much interest - it can be fixed at any point. Most important thing is to ensure that API feels right, documentation feels clear and people are in general comfortable with using proposed modules as they are. I will do more in-depth review but it will _all_ be about API and docs and naming.
Jun 13 2015
parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/13/15 11:49 PM, Dicebot wrote:
 On Sunday, 14 June 2015 at 00:24:51 UTC, Andrei Alexandrescu wrote:
 So we have:

 * 1 request to change names;
 * 3 requests to wank around the directory structure;
 * 0 of everything else.

 Sigh.
That is to be expected and intended for formal Phobos review. Implementation is not of much interest - it can be fixed at any point. Most important thing is to ensure that API feels right, documentation feels clear and people are in general comfortable with using proposed modules as they are. I will do more in-depth review but it will _all_ be about API and docs and naming.
Suggestions for better names are welcome as addenda, and I will act on some, but they're no substitute for competent reviews. We need as a community to learn how to do good reviews. Anyone can put a finger on a name of a thing and say they like another name better. ANYONE. Real review of a library is figuring out how well the proposed library's abstractions fulfill its charter and intended use cases. Andrei
Jun 14 2015
prev sibling next sibling parent Jacob Carlborg <doob me.com> writes:
On 2015-06-14 02:24, Andrei Alexandrescu wrote:

 * 1 request to change names;
 * 3 requests to wank around the directory structure;
 * 0 of everything else.

 Sigh.
These are things that are easy to see right away, without looking deep inside the code. You can interpret this as: 1. Either your implementation is already good enough 2. Or people haven't started to look at the implementation yet and are commenting on the things that they is right away Count me on the request to change the directory structure :) -- /Jacob Carlborg
Jun 14 2015
prev sibling parent reply "Brian Schott" <briancschott gmail.com> writes:
On Sunday, 14 June 2015 at 00:24:51 UTC, Andrei Alexandrescu 
wrote:
 So we have:

 * 1 request to change names;
 * 3 requests to wank around the directory structure;
 * 0 of everything else.

 Sigh.


 Andrei
How about this: AllocatorList.allocate() calls AllocatorList.owns(), but only if you don't compile with -release. Check the assert statement. AllocatorList.owns() is non-const, so the for loops of these two functions start rearranging the root pointers and end up in an infinite loop in debug builds.
Jun 14 2015
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/14/15 4:00 AM, Brian Schott wrote:
 On Sunday, 14 June 2015 at 00:24:51 UTC, Andrei Alexandrescu wrote:
 So we have:

 * 1 request to change names;
 * 3 requests to wank around the directory structure;
 * 0 of everything else.

 Sigh.


 Andrei
How about this: AllocatorList.allocate() calls AllocatorList.owns(), but only if you don't compile with -release. Check the assert statement. AllocatorList.owns() is non-const, so the for loops of these two functions start rearranging the root pointers and end up in an infinite loop in debug builds.
Interesting. Do you have a repro for this? Thanks! -- Andrei
Jun 14 2015
parent reply "Brian Schott" <briancschott gmail.com> writes:
On Sunday, 14 June 2015 at 14:30:00 UTC, Andrei Alexandrescu 
wrote:
 Interesting. Do you have a repro for this? Thanks! -- Andrei
Not yet, but while looking for one I found that this code triggers an assertion in Region.expand() ``` module test; import std.experimental.allocator; import std.stdio; private alias AllocatorType = CAllocatorImpl!(AllocatorList!( n => Region!Mallocator(1024 * 4), NullAllocator)); struct Big { ubyte[1024] bytes; } void main(string[] args){ auto a = new AllocatorType; foreach (i; 0 .. 100) { a.make!Big(); writeln(i); } } ```
Jun 14 2015
parent "Brian Schott" <briancschott gmail.com> writes:
On Sunday, 14 June 2015 at 22:47:59 UTC, Brian Schott wrote:
 Not yet, but while looking for one I found that this code 
 triggers an assertion in Region.expand()
Running the above code with "GCAllocator" substituted for "NullAllocator" causes an InvalidMemoryOperationError after all 100 iterations of the loop have completed.
Jun 15 2015
prev sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/13/15 4:16 PM, ZombineDev 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 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.
Moved a bunch of these into building_blocks: https://github.com/andralex/phobos/commit/14ccc3a02f3dd332cb435abaf3c35cb8847797c0 However, I kept gc_allocator, mallocator, and mmap_allocator outside of it as "core" allocator that doesn't depend on any other. Andrei
Jun 21 2015
parent "Matthias Bentrup" <matthias.bentrup googlemail.com> writes:
Can I use these allocators in  nogc code too ?
Jun 22 2015
prev sibling parent reply "Baz" <bb.temp gmx.com> writes:
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_
uilding_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
Jun 12 2015
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
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
Jun 12 2015
parent "Baz" <bb.temp gmx.com> writes:
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 ?
Jun 12 2015
prev sibling next sibling parent "Dicebot" <public dicebot.lv> writes:
Also wanted to apologize for authors of any other Phobos 
proposals - I did intend to step down from review manager role 
but this specific package is of huge personal interest to me, 
thus the exception has been made.
Jun 12 2015
prev sibling next sibling parent "Andrea Fontana" <nospam example.com> writes:
On Friday, 12 June 2015 at 11:06:43 UTC, 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

 This proposal will undergo the review/inclusion process similar 
 to std.experimental.logger :

 1. 2 week review/discussion period
 2. if proposal author feels confident, 2 week voting period for 
 accepting into sts.experimental
 3. once enough field trial feedback has been accumulated, vote 
 for moving into std mainline during the upcoming release beta 
 period

 If reviewing find some fundamental issues or voting will fail, 
 the whole process will repeats again per proposal authors 
 request.

 Please start destruction, review period ends on 26.06 (23.59 +0 
 Greenwich)
Great work :) It would be nice if someone could write some tests/benchmarks in order to show for each allocator where it works well over others.
Jun 12 2015
prev sibling next sibling parent reply Jacob Carlborg <doob me.com> writes:
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
As I said in the other thread: * Andrei's ODBC code seems to be in the same branch * Some code is commented in std.math and std.traits * What is std.typed_allocator? It doesn't seem to have anything related to allocators, some leftover code? -- /Jacob Carlborg
Jun 12 2015
parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/12/15 7:31 AM, Jacob Carlborg wrote:
 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
As I said in the other thread: * Andrei's ODBC code seems to be in the same branch * Some code is commented in std.math and std.traits * What is std.typed_allocator? It doesn't seem to have anything related to allocators, some leftover code?
Sorry about that. I now created a pull request with that stuff taken care of: https://github.com/D-Programming-Language/phobos/pull/3405 Andrei
Jun 12 2015
prev sibling next sibling parent reply Jacob Carlborg <doob me.com> writes:
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
Jun 13 2015
parent reply "Andrei Alexandrescu" <SeeWebsiteForEmail erdani.org> writes:
On Saturday, 13 June 2015 at 19:08:26 UTC, Jacob Carlborg wrote:
 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.
https://github.com/D-Programming-Language/phobos/commit/319f3297418c515a6d2e 2e6e52d0f3f5895f587 changes .it to .instance for all allocators. -- Andrei
Jun 16 2015
next sibling parent Jacob Carlborg <doob me.com> writes:
On 2015-06-16 22:29, Andrei Alexandrescu wrote:

 https://github.com/D-Programming-Language/phobos/commit/319f3297418c515a6d2e52e6e52d0f3f5895f587
 changes .it to .instance for all allocators. -- Andrei
Awesome, thanks. -- /Jacob Carlborg
Jun 16 2015
prev sibling parent bitwise <bitwise.pvt gmail.com> writes:
On Tue, 16 Jun 2015 16:29:08 -0400, Andrei Alexandrescu  
<SeeWebsiteForEmail erdani.org> wrote:

 On Saturday, 13 June 2015 at 19:08:26 UTC, Jacob Carlborg wrote:
 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.
https://github.com/D-Programming-Language/phobos/commit/319f3297418c515a6d2e 2e6e52d0f3f5895f587 changes .it to .instance for all allocators. -- Andrei
Why not .that? ;) Bit
Jun 20 2015
prev sibling next sibling parent "Dicebot" <public dicebot.lv> writes:
Bringing back to first page.
Jun 19 2015
prev sibling next sibling parent reply "Dicebot" <public dicebot.lv> writes:
Ok, as there has not been much attention here in last days, I 
will put a short summary of my own.

In general, I believe this is extremely high quality proposal and 
Andrei stands up to his reputation. The very design seems to fit 
with D idiomatics and it may change completely how people think 
about interaction with allocator library. I like that is focuses 
on robust API for building allocators instead of more "magic" to 
use those. That said, reviewing actual implementation quality is 
impossible until I have a chance to try it in real large scale 
project and I will not be trying to do that, assuming that Andrei 
is incapable of writing something completely terrible even if he 
tried to.

Pretty much all of my concerns are about documentation, API and 
clarity of intention. Quality of implementation won't matter if 
developers won't have a clear understanding on how to use it and 
existing proposals feels overwhelming unless one is willing to 
commit quite some time to research it.

Some notes:

1. I have already mentioned that there is neither module 
structure overview in `package.d` nor actual module structure. 
This is still the case for 
http://erdani.com/d/phobos-prerelease/std_experimental_allocator.html
2. `IAllocator` is defined inside `package.d` file. That means 
that it is impossible to use interface without import ALL of 
allocator modules
3. Same concern is about 
https://github.com/andralex/phobos/blob/allocator/std/experimental/allocator
package.d#L218-L228 - unless you actually import all stuff via package.d, those
configured allocators are not available.
4. There are no higher level usage examples and/or guidelines 
about how this is supposed to fit in user applications. Intention 
behind the library may be familiar to users coming from C++ but 
target D audience is much more than that. Having 
std.allocator.showcase is nice but it is still a bit too 
theoretical.
5. 
http://erdani.com/d/phobos-prerelease/std_experimental_allocator_
tats_collector.html has no overview documentation at all
6. Usage of ternary is not always clear / justified. In 
`IAllocator` it is explained and makes sense but there are things 
like 
http://erdani.com/d/phobos-prerelease/std_experimental_allocator_
itmapped_block.html ("Ternary empty() - Returns true iff no memory is currently
allocated with this allocator"). I am still not sure why it is used there
instead of simple boolean.

Overall opinion : I would surely vote for inclusion of this 
proposal in Phobos but in current shape it is not something I'd 
recommend to try to beginner/intermediate level D user.
Jun 21 2015
next sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/21/15 4:47 AM, Dicebot wrote:
 1. I have already mentioned that there is neither module structure
 overview in `package.d` nor actual module structure. This is still the
 case for
 http://erdani.com/d/phobos-prerelease/std_experimental_allocator.html
 2. `IAllocator` is defined inside `package.d` file. That means that it
 is impossible to use interface without import ALL of allocator modules
 3. Same concern is about
 https://github.com/andralex/phobos/blob/allocator/std/experimental/allocator/package.d#L218-L228
 - unless you actually import all stuff via package.d, those configured
 allocators are not available.
 4. There are no higher level usage examples and/or guidelines about how
 this is supposed to fit in user applications. Intention behind the
 library may be familiar to users coming from C++ but target D audience
 is much more than that. Having std.allocator.showcase is nice but it is
 still a bit too theoretical.
 5.
 http://erdani.com/d/phobos-prerelease/std_experimental_allocator_stats_collector.html
 has no overview documentation at all
 6. Usage of ternary is not always clear / justified. In `IAllocator` it
 is explained and makes sense but there are things like
 http://erdani.com/d/phobos-prerelease/std_experimental_allocator_bitmapped_block.html
 ("Ternary empty() - Returns true iff no memory is currently allocated
 with this allocator"). I am still not sure why it is used there instead
 of simple boolean.

 Overall opinion : I would surely vote for inclusion of this proposal in
 Phobos but in current shape it is not something I'd recommend to try to
 beginner/intermediate level D user.
Thanks for the feedback. I'll get to work on this straight away. -- Andrei
Jun 21 2015
prev sibling next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/21/15 4:47 AM, Dicebot wrote:
 1. I have already mentioned that there is neither module structure
 overview in `package.d` nor actual module structure. This is still the
 case for
 http://erdani.com/d/phobos-prerelease/std_experimental_allocator.html
Improved: http://erdani.com/d/phobos-prerelease/std_experimental_allocator.html
 2. `IAllocator` is defined inside `package.d` file. That means that it
 is impossible to use interface without import ALL of allocator modules
Fixed, now interested users need to import std.experimental.allocator.building_blocks.
 3. Same concern is about
 https://github.com/andralex/phobos/blob/allocator/std/experimental/allocator/package.d#L218-L228
 - unless you actually import all stuff via package.d, those configured
 allocators are not available.
Fixed per above.
 4. There are no higher level usage examples and/or guidelines about how
 this is supposed to fit in user applications. Intention behind the
 library may be familiar to users coming from C++ but target D audience
 is much more than that. Having std.allocator.showcase is nice but it is
 still a bit too theoretical.
Coming soon.
 5.
 http://erdani.com/d/phobos-prerelease/std_experimental_allocator_stats_collector.html
 has no overview documentation at all
Coming soon.
 6. Usage of ternary is not always clear / justified. In `IAllocator` it
 is explained and makes sense but there are things like
 http://erdani.com/d/phobos-prerelease/std_experimental_allocator_bitmapped_block.html
 ("Ternary empty() - Returns true iff no memory is currently allocated
 with this allocator"). I am still not sure why it is used there instead
 of simple boolean.
Coming soon. Andrei
Jun 22 2015
parent reply "Dicebot" <public dicebot.lv> writes:
On Monday, 22 June 2015 at 19:51:50 UTC, Andrei Alexandrescu 
wrote:
 2. `IAllocator` is defined inside `package.d` file. That means 
 that it
 is impossible to use interface without import ALL of allocator 
 modules
Fixed, now interested users need to import std.experimental.allocator.building_blocks.
 3. Same concern is about
 https://github.com/andralex/phobos/blob/allocator/std/experimental/allocator/package.d#L218-L228
 - unless you actually import all stuff via package.d, those 
 configured
 allocators are not available.
Fixed per above.
Is it? I still see those symbols defined in package.d , 14ccc3a02f3dd332cb435abaf3c35cb8847797c0 does not seem to have affected that.
Jun 22 2015
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/22/15 2:40 PM, Dicebot wrote:
 On Monday, 22 June 2015 at 19:51:50 UTC, Andrei Alexandrescu wrote:
 2. `IAllocator` is defined inside `package.d` file. That means that it
 is impossible to use interface without import ALL of allocator modules
Fixed, now interested users need to import std.experimental.allocator.building_blocks.
 3. Same concern is about
 https://github.com/andralex/phobos/blob/allocator/std/experimental/allocator/package.d#L218-L228

 - unless you actually import all stuff via package.d, those configured
 allocators are not available.
Fixed per above.
Is it? I still see those symbols defined in package.d , 14ccc3a02f3dd332cb435abaf3c35cb8847797c0 does not seem to have affected that.
Perhaps I misunderstood the request - currently the imports in allocator/package.d are: public import std.experimental.allocator.common, std.experimental.allocator.typed; import std.algorithm, std.conv, std.exception, std.range, std.traits, std.typecons, std.typetuple; version(unittest) import std.random, std.stdio; Is that okay, and if not what should change? Andrei
Jun 22 2015
parent reply "Dicebot" <public dicebot.lv> writes:
On Monday, 22 June 2015 at 22:38:19 UTC, Andrei Alexandrescu 
wrote:
 Perhaps I misunderstood the request - currently the imports in 
 allocator/package.d are:

 public import std.experimental.allocator.common,
     std.experimental.allocator.typed;
 import std.algorithm, std.conv, std.exception, std.range, 
 std.traits,
     std.typecons, std.typetuple;
 version(unittest) import std.random, std.stdio;

 Is that okay, and if not what should change?
My concern was about the fact that symbols `IAllocator`, `theAllocator`, `processAllocator` and bunch of others are defined within `package.d` itself and not provided via public import. That means that anyone willing to change default allocator or use `make` MUST import all std.allocator modules even if nothing else is really needed (those utilities look very independent). Which means processing bunch of unnecessary imports - and more of those if more modules will get added to the package. I'd prefer to have a small dedicated module, i.e. `std.allocator.api` (not going to discuss names!) and do public import of it from `std.allocator.package.d`. Makes sense?
Jun 23 2015
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/23/15 2:18 AM, Dicebot wrote:
 On Monday, 22 June 2015 at 22:38:19 UTC, Andrei Alexandrescu wrote:
 Perhaps I misunderstood the request - currently the imports in
 allocator/package.d are:

 public import std.experimental.allocator.common,
 std.experimental.allocator.typed; import std.algorithm, std.conv,
 std.exception, std.range, std.traits, std.typecons, std.typetuple;
 version(unittest) import std.random, std.stdio;

 Is that okay, and if not what should change?
My concern was about the fact that symbols `IAllocator`, `theAllocator`, `processAllocator` and bunch of others are defined within `package.d` itself and not provided via public import. That means that anyone willing to change default allocator or use `make` MUST import all std.allocator modules even if nothing else is really needed (those utilities look very independent). Which means processing bunch of unnecessary imports - and more of those if more modules will get added to the package. I'd prefer to have a small dedicated module, i.e. `std.allocator.api` (not going to discuss names!) and do public import of it from `std.allocator.package.d`. Makes sense?
I see. Well this raises the question whether importing std.xyz automatically means everything under std.xyz is transitorily imported. Right now it's not - the more advanced/obscure building blocks are not automatically imported if you just import std.allocator; for those you'd need to import the std.allocator.building_blocks package, or individual modules from it like std.allocator.building_blocks.region. This idea was proposed by Mike on 2015/05/18:
 Would it be better to move the porcelain code into package.d so the
 nomenclature simply becomes `import std.allocator;`?
I liked the idea that someone who's not an expert goes "let me import std.allocator and call it a day" whereas someone who wants to tweak, customize, etc. they'd need to be more specific. If we define an entry point such as "std.allocator.api", novices will have one more thing to remember, or some will still import "std.allocator" thus pulling everything without realizing it. So I'd say let's keep simple things simple. If you want to allocate, import std.allocator. Andrei
Jun 23 2015
next sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/23/15 8:56 AM, Andrei Alexandrescu wrote:
 I see. Well this raises the question whether importing std.xyz
 automatically means everything under std.xyz is transitorily imported.
s/transitorily/transitively/
Jun 23 2015
prev sibling parent reply "extrawurst" <stephan extrawurst.org> writes:
On Tuesday, 23 June 2015 at 15:56:15 UTC, Andrei Alexandrescu 
wrote:
 On 6/23/15 2:18 AM, Dicebot wrote:
 On Monday, 22 June 2015 at 22:38:19 UTC, Andrei Alexandrescu 
 wrote:
 Perhaps I misunderstood the request - currently the imports in
 allocator/package.d are:

 public import std.experimental.allocator.common,
 std.experimental.allocator.typed; import std.algorithm, 
 std.conv,
 std.exception, std.range, std.traits, std.typecons, 
 std.typetuple;
 version(unittest) import std.random, std.stdio;

 Is that okay, and if not what should change?
My concern was about the fact that symbols `IAllocator`, `theAllocator`, `processAllocator` and bunch of others are defined within `package.d` itself and not provided via public import. That means that anyone willing to change default allocator or use `make` MUST import all std.allocator modules even if nothing else is really needed (those utilities look very independent). Which means processing bunch of unnecessary imports - and more of those if more modules will get added to the package. I'd prefer to have a small dedicated module, i.e. `std.allocator.api` (not going to discuss names!) and do public import of it from `std.allocator.package.d`. Makes sense?
I see. Well this raises the question whether importing std.xyz automatically means everything under std.xyz is transitorily imported. Right now it's not - the more advanced/obscure building blocks are not automatically imported if you just import std.allocator; for those you'd need to import the std.allocator.building_blocks package, or individual modules from it like std.allocator.building_blocks.region. This idea was proposed by Mike on 2015/05/18:
 Would it be better to move the porcelain code into package.d 
 so the
 nomenclature simply becomes `import std.allocator;`?
I liked the idea that someone who's not an expert goes "let me import std.allocator and call it a day" whereas someone who wants to tweak, customize, etc. they'd need to be more specific. If we define an entry point such as "std.allocator.api", novices will have one more thing to remember, or some will still import "std.allocator" thus pulling everything without realizing it. So I'd say let's keep simple things simple. If you want to allocate, import std.allocator. Andrei
I agree with Adam on this: "Just a quick concern, I don't think a package.d should ever have anything except imports in it" (see http://forum.dlang.org/post/qwatonmpnoyjsvzjpyjl forum.dlang.org) for convenience the package.d could still import a bunch of default stuff publicly for the "normal" user.
Jun 23 2015
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/23/15 9:48 AM, extrawurst wrote:
 I agree with Adam on this: "Just a quick concern, I don't think a
 package.d should ever have anything except imports in it"
 (see http://forum.dlang.org/post/qwatonmpnoyjsvzjpyjl forum.dlang.org)
What is the rationale? -- Andrei
Jun 23 2015
next sibling parent reply "extrawurst" <stephan extrawurst.org> writes:
On Tuesday, 23 June 2015 at 16:49:45 UTC, Andrei Alexandrescu 
wrote:
 On 6/23/15 9:48 AM, extrawurst wrote:
 I agree with Adam on this: "Just a quick concern, I don't 
 think a
 package.d should ever have anything except imports in it"
 (see 
 http://forum.dlang.org/post/qwatonmpnoyjsvzjpyjl forum.dlang.org)
What is the rationale? -- Andrei
I simply quote his following post in the mentioned thread: "The biggest benefit of breaking up the big modules is so you can access some of it without requiring all of it. But when the part you want is in the package.d, you can't get it independently anymore; importing that also imports everything else, negating the reason it was split up in the first place."
Jun 23 2015
next sibling parent =?UTF-8?B?U8O2bmtlIEx1ZHdpZw==?= <sludwig rejectedsoftware.com> writes:
Am 23.06.2015 um 18:53 schrieb extrawurst:
 On Tuesday, 23 June 2015 at 16:49:45 UTC, Andrei Alexandrescu wrote:
 On 6/23/15 9:48 AM, extrawurst wrote:
 I agree with Adam on this: "Just a quick concern, I don't think a
 package.d should ever have anything except imports in it"
 (see http://forum.dlang.org/post/qwatonmpnoyjsvzjpyjl forum.dlang.org)
What is the rationale? -- Andrei
I simply quote his following post in the mentioned thread: "The biggest benefit of breaking up the big modules is so you can access some of it without requiring all of it. But when the part you want is in the package.d, you can't get it independently anymore; importing that also imports everything else, negating the reason it was split up in the first place."
Not to mention https://issues.dlang.org/show_bug.cgi?id=11847
Jun 23 2015
prev sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/23/15 9:53 AM, extrawurst wrote:
 On Tuesday, 23 June 2015 at 16:49:45 UTC, Andrei Alexandrescu wrote:
 On 6/23/15 9:48 AM, extrawurst wrote:
 I agree with Adam on this: "Just a quick concern, I don't think a
 package.d should ever have anything except imports in it"
 (see http://forum.dlang.org/post/qwatonmpnoyjsvzjpyjl forum.dlang.org)
What is the rationale? -- Andrei
I simply quote his following post in the mentioned thread: "The biggest benefit of breaking up the big modules is so you can access some of it without requiring all of it. But when the part you want is in the package.d, you can't get it independently anymore; importing that also imports everything else, negating the reason it was split up in the first place."
But that doesn't apply to packages that do NOT originate as big modules, so they have no backward compatibility issue. This is the case for std.allocator. I see a net pessimization for everyone involved to change the current packaging. All I'm saying is we shouldn't take it noncritically that packages should be done one particular way. Andrei
Jun 23 2015
parent reply "Adam D. Ruppe" <destructionator gmail.com> writes:
On Tuesday, 23 June 2015 at 16:56:55 UTC, Andrei Alexandrescu 
wrote:
 But that doesn't apply to packages that do NOT originate as big 
 modules, so they have no backward compatibility issue.
My thought isn't really about backward compatibility but about minimizing dependencies with sibling modules. I don't want to repeat my argument too much from the other thread, but imagine you're writing a minimalist library that is meant to interact with other minimalist libraries. To interact, you want a shared interface and basic types. But to be minimalist, you want to pull as little other standard code as possible. The typical user might just import std.whatever and get it all available. But this minimalist user only wants std.whatever.basic_interface. If the basic interface is shoved inside package.d, she can't get get to it without inadvertently pulling in more modules too. This can quickly grow into a web of dependencies where importing just an interface definition ends up grabbing dozens if implementations you don't want too, bloating compile times and binary sizes.
Jun 23 2015
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/23/15 10:15 AM, Adam D. Ruppe wrote:
 On Tuesday, 23 June 2015 at 16:56:55 UTC, Andrei Alexandrescu wrote:
 But that doesn't apply to packages that do NOT originate as big
 modules, so they have no backward compatibility issue.
My thought isn't really about backward compatibility but about minimizing dependencies with sibling modules. I don't want to repeat my argument too much from the other thread, but imagine you're writing a minimalist library that is meant to interact with other minimalist libraries. To interact, you want a shared interface and basic types. But to be minimalist, you want to pull as little other standard code as possible. The typical user might just import std.whatever and get it all available. But this minimalist user only wants std.whatever.basic_interface. If the basic interface is shoved inside package.d, she can't get get to it without inadvertently pulling in more modules too. This can quickly grow into a web of dependencies where importing just an interface definition ends up grabbing dozens if implementations you don't want too, bloating compile times and binary sizes.
The case with std.allocator is not everything is imported in it, so no bloating no nothing. -- Andrei
Jun 23 2015
parent "ponce" <contact gam3sfrommars.fr> writes:
On Tuesday, 23 June 2015 at 19:16:33 UTC, Andrei Alexandrescu 
wrote:
 The case with std.allocator is not everything is imported in 
 it, so no bloating no nothing. -- Andrei
My assumption with D libraries is that "import std.thing;" imports everything in the the whole package. Not that I've a strong opinion on it either.
Jun 23 2015
prev sibling parent reply "Dicebot" <public dicebot.lv> writes:
On Tuesday, 23 June 2015 at 16:49:45 UTC, Andrei Alexandrescu 
wrote:
 On 6/23/15 9:48 AM, extrawurst wrote:
 I agree with Adam on this: "Just a quick concern, I don't 
 think a
 package.d should ever have anything except imports in it"
 (see 
 http://forum.dlang.org/post/qwatonmpnoyjsvzjpyjl forum.dlang.org)
What is the rationale? -- Andrei
My reasoning is simple : when `package.d` only contains public imports it allows to both use simple `import std.allocator` for those who want to get started quickly and pick only necessary imports for those who try optimizing build times / code clarity. Not doing that does not change anything for those who prefer `import std.allocator` but makes fine tuning of imports impossible. Thus former approach looks either equal or superior for all use cases.
Jun 23 2015
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/23/15 10:16 AM, Dicebot wrote:
 On Tuesday, 23 June 2015 at 16:49:45 UTC, Andrei Alexandrescu wrote:
 On 6/23/15 9:48 AM, extrawurst wrote:
 I agree with Adam on this: "Just a quick concern, I don't think a
 package.d should ever have anything except imports in it"
 (see http://forum.dlang.org/post/qwatonmpnoyjsvzjpyjl forum.dlang.org)
What is the rationale? -- Andrei
My reasoning is simple : when `package.d` only contains public imports it allows to both use simple `import std.allocator` for those who want to get started quickly and pick only necessary imports for those who try optimizing build times / code clarity. Not doing that does not change anything for those who prefer `import std.allocator` but makes fine tuning of imports impossible. Thus former approach looks either equal or superior for all use cases.
The "import std.allocator" is already minimal - only contains the high level stuff. -- Andrei
Jun 23 2015
parent reply "Dicebot" <public dicebot.lv> writes:
On Tuesday, 23 June 2015 at 19:17:21 UTC, Andrei Alexandrescu 
wrote:
 On 6/23/15 10:16 AM, Dicebot wrote:
 On Tuesday, 23 June 2015 at 16:49:45 UTC, Andrei Alexandrescu 
 wrote:
 On 6/23/15 9:48 AM, extrawurst wrote:
 I agree with Adam on this: "Just a quick concern, I don't 
 think a
 package.d should ever have anything except imports in it"
 (see 
 http://forum.dlang.org/post/qwatonmpnoyjsvzjpyjl forum.dlang.org)
What is the rationale? -- Andrei
My reasoning is simple : when `package.d` only contains public imports it allows to both use simple `import std.allocator` for those who want to get started quickly and pick only necessary imports for those who try optimizing build times / code clarity. Not doing that does not change anything for those who prefer `import std.allocator` but makes fine tuning of imports impossible. Thus former approach looks either equal or superior for all use cases.
The "import std.allocator" is already minimal - only contains the high level stuff. -- Andrei
So you have chosen worst of both worlds - neither give power users ability to fine tune imports nor allow casual users always go with `import std.allocator` and be happy? :) If anything, that will be the first package.d in Phobos (AFAIK) which won't feature public import of _all_ package modules.
Jun 23 2015
next sibling parent reply "Tofu Ninja" <emmons0 purdue.edu> writes:
On Tuesday, 23 June 2015 at 21:12:10 UTC, Dicebot wrote:
 So you have chosen worst of both worlds - neither give power 
 users ability to fine tune imports nor allow casual users 
 always go with `import std.allocator` and be happy? :)

 If anything, that will be the first package.d in Phobos (AFAIK) 
 which won't feature public import of _all_ package modules.
I agree, a minimalist package.d seems utterly pointless to me.
Jun 23 2015
parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/23/15 2:14 PM, Tofu Ninja wrote:
 On Tuesday, 23 June 2015 at 21:12:10 UTC, Dicebot wrote:
 So you have chosen worst of both worlds - neither give power users
 ability to fine tune imports nor allow casual users always go with
 `import std.allocator` and be happy? :)

 If anything, that will be the first package.d in Phobos (AFAIK) which
 won't feature public import of _all_ package modules.
I agree, a minimalist package.d seems utterly pointless to me.
It's not minimalist. -- Andrei
Jun 23 2015
prev sibling next sibling parent reply Dmitry Olshansky <dmitry.olsh gmail.com> writes:
On 24-Jun-2015 00:12, Dicebot wrote:
 On Tuesday, 23 June 2015 at 19:17:21 UTC, Andrei Alexandrescu wrote:
 On 6/23/15 10:16 AM, Dicebot wrote:
 On Tuesday, 23 June 2015 at 16:49:45 UTC, Andrei Alexandrescu wrote:
 On 6/23/15 9:48 AM, extrawurst wrote:
 I agree with Adam on this: "Just a quick concern, I don't think a
 package.d should ever have anything except imports in it"
 (see http://forum.dlang.org/post/qwatonmpnoyjsvzjpyjl forum.dlang.org)
What is the rationale? -- Andrei
My reasoning is simple : when `package.d` only contains public imports it allows to both use simple `import std.allocator` for those who want to get started quickly and pick only necessary imports for those who try optimizing build times / code clarity. Not doing that does not change anything for those who prefer `import std.allocator` but makes fine tuning of imports impossible. Thus former approach looks either equal or superior for all use cases.
The "import std.allocator" is already minimal - only contains the high level stuff. -- Andrei
So you have chosen worst of both worlds - neither give power users ability to fine tune imports nor allow casual users always go with `import std.allocator` and be happy? :)
 If anything, that will be the first package.d in Phobos (AFAIK) which
 won't feature public import of _all_ package modules.
I'm not sure if that's the case with std.allocator but importing package IMHO should import _typical_ set of submodules. Things that are more niche and rare (power user oriented) shouldn't really be in package.d -- Dmitry Olshansky
Jun 23 2015
parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/23/15 2:29 PM, Dmitry Olshansky wrote:
 I'm not sure if that's the case with std.allocator but importing package
 IMHO should import _typical_ set of submodules.

 Things that are more niche and rare (power user oriented) shouldn't
 really be in package.d
Yah, that attitude describes std.allocator well. -- Andrei
Jun 23 2015
prev sibling next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/23/15 2:12 PM, Dicebot wrote:
 So you have chosen worst of both worlds - neither give power users
 ability to fine tune imports nor allow casual users always go with
 `import std.allocator` and be happy? :)
There is functionality in std.allocator to get anyone started who doesn't want to sit down and define their own allocator. People who want to do that can import std.allocator.building_blocks which is a package fully including all that's needed.
 If anything, that will be the first package.d in Phobos (AFAIK) which
 won't feature public import of _all_ package modules.
I'm not afraid of creating precedent if that's the best way to go. Again: I have the impression we're blocked into an assumption we didn't take critically until now. Andrei
Jun 23 2015
next sibling parent reply Steven Schveighoffer <schveiguy yahoo.com> writes:
On 6/23/15 6:07 PM, Andrei Alexandrescu wrote:
 On 6/23/15 2:12 PM, Dicebot wrote:
 So you have chosen worst of both worlds - neither give power users
 ability to fine tune imports nor allow casual users always go with
 `import std.allocator` and be happy? :)
There is functionality in std.allocator to get anyone started who doesn't want to sit down and define their own allocator. People who want to do that can import std.allocator.building_blocks which is a package fully including all that's needed.
 If anything, that will be the first package.d in Phobos (AFAIK) which
 won't feature public import of _all_ package modules.
I'm not afraid of creating precedent if that's the best way to go. Again: I have the impression we're blocked into an assumption we didn't take critically until now.
What happened basically was: 1. There's too much crap in some modules, it's hard to maintain, hard to process. 2. Let's split it up! Oh, but some people want to be able to import everything a la java's import java.io.*, and existing code!!! 3. package.d is the solution, just import that and it's the same thing! I personally think that if we want to define package.d as "importing all the submodules", then we should stick to that. If we want to go another direction, we need to redo all the other splitting of modules we have done so far, as that is the pattern. I'd recommend std.allocator.basic instead of std.allocator for a basic direction. If it doesn't make sense to import ALL of std.allocator ever, then drop package.d. But you don't have to listen to me :) -Steve
Jun 23 2015
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/23/15 3:17 PM, Steven Schveighoffer wrote:
 we need to redo
no
Jun 23 2015
parent Steven Schveighoffer <schveiguy yahoo.com> writes:
On 6/23/15 6:18 PM, Andrei Alexandrescu wrote:
 On 6/23/15 3:17 PM, Steven Schveighoffer wrote:
 we need to redo
no
Yeah, so then it becomes import std.somepackage means import all the modules in that package, except for std.allocator. But that's fine, if we want that type of inconsistency, we can just RTFM the newbies. -Steve
Jun 23 2015
prev sibling parent "Dicebot" <public dicebot.lv> writes:
Ok, if you feel so strongly about it, let's not waste time in 
arguing. You are not obliged to agree to any of reviewer comments 
so as long as everyone is clear about their opinion, the eventual 
voting will decide. I am simply pointing out things that catch my 
attention.
Jun 23 2015
prev sibling parent reply "Mike" <none none.com> writes:
On Tuesday, 23 June 2015 at 21:12:10 UTC, Dicebot wrote:
 On Tuesday, 23 June 2015 at 19:17:21 UTC, Andrei Alexandrescu
 The "import std.allocator" is already minimal - only contains 
 the high level stuff. -- Andrei
So you have chosen worst of both worlds - neither give power users ability to fine tune imports nor allow casual users always go with `import std.allocator` and be happy? :) If anything, that will be the first package.d in Phobos (AFAIK) which won't feature public import of _all_ package modules.
Well, since it was apparently my lousy suggestion that led to this, I guess I'm obligated to comment. It seems there is already precedent with std.digest.digest (http://dlang.org/phobos/std_digest_digest.html). I'm sorry I wasn't aware of this at the time. std.allocator.porcelain couldn't stand, and I suggested what seemed most natural to me. I've spend a number of years in C#-land, and that of course has an affect on my biases. But D is not C# (thankfully). Following the std.digest.digest precedent, there should probably be std.allocator.allocator. It stinks IMO (std.digiest.api and std.allocator.api would have been much better), but the naming discussions are starting to take their toll. Anyway, I suggest std.allocator.allocator as a compromise and a precedent to follow in the future. It's tolerable. Also, I can't seem to navigate std.allocator tree in the left nav at http://erdani.com/d/phobos-prerelease/std_experimental_allocator.html. Please advise or fix. Mike
Jun 23 2015
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/23/15 6:11 PM, Mike wrote:
 It seems there is already precedent with std.digest.digest
 (http://dlang.org/phobos/std_digest_digest.html).  I'm sorry I wasn't
 aware of this at the time.  std.allocator.porcelain couldn't stand, and
 I suggested what seemed most natural to me. I've spend a number of years
 in C#-land, and that of course has an affect on my biases.  But D is not
 C# (thankfully).
Well it's about time we engineers stop suspending our sense of ridiculousness when looking at programmer names. Once some minimal sense and sensibility is in action, it's clear that something like std.digest.digest.digest is just ridiculous. That shouldn't have passed review, and today it shouldn't be used as a precedent.
 Following the std.digest.digest precedent, there should probably be
 std.allocator.allocator.  It stinks IMO (std.digiest.api and
 std.allocator.api would have been much better), but the naming
 discussions are starting to take their toll.
std.allocator.allocator.IAllocator std.allocator.allocator.theAllocator; Yep, "ridiculous" is the first thing that comes to mind. I find it difficult to digest (ehm) the fact that the same community that thinks "std.allocator" is just not going to cut the mustard, simultaneously believes "std.allocator.allocator" is a good idea.
 Anyway, I suggest std.allocator.allocator as a compromise and a
 precedent to follow in the future.  It's tolerable.

 Also, I can't seem to navigate std.allocator tree in the left nav at
 http://erdani.com/d/phobos-prerelease/std_experimental_allocator.html.
 Please advise or fix.
Yah, noticed that too. Will look into it tomorrow. Andrei
Jun 23 2015
parent "Mike" <none none.com> writes:
On Wednesday, 24 June 2015 at 04:00:11 UTC, Andrei Alexandrescu 
wrote:
 std.allocator.allocator.IAllocator 
 std.allocator.allocator.theAllocator;

 Yep, "ridiculous" is the first thing that comes to mind.

 I find it difficult to digest (ehm) the fact that the same 
 community that thinks "std.allocator" is just not going to cut 
 the mustard, simultaneously believes "std.allocator.allocator" 
 is a good idea.
Of course you are right. It was indeed a poor suggestion. But it's the only example I could find where a separate module name was used to declare the high-level API. Perhaps it was even written before package.d existed. The case against std.allocator isn't about the name, but about the fact that it doesn't do what people, apparently, expect: import the entire public API. I count only 4 uses of package.d in official Phobos: https://github.com/D-Programming-Language/phobos/blob/master/std/algorithm/package.d https://github.com/D-Programming-Language/phobos/blob/master/std/container/package.d https://github.com/D-Programming-Language/phobos/blob/master/std/range/package.d https://github.com/D-Programming-Language/phobos/blob/master/std/regex/package.d All of them seem to import the entire public API, but only std.algorithm.package.d is exclusively used for public imports. The others all have some definition of high-level API features. So, I don't see a precedent one way or another. And maybe that's how it should be: defined based on the intended use of the package. Unfortunately, those against using package.d for the high-level API, did not seem to offer a suggestion for where to put your porcelain. So, if you wish to accommodate them and need a name, consider std.allocator.api. Mike
Jun 24 2015
prev sibling next sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/21/15 4:47 AM, Dicebot wrote:
 4. There are no higher level usage examples and/or guidelines about how
 this is supposed to fit in user applications. Intention behind the
 library may be familiar to users coming from C++ but target D audience
 is much more than that. Having std.allocator.showcase is nice but it is
 still a bit too theoretical.
Done: https://github.com/D-Programming-Language/phobos/commit/fd7689ef1369e55252d2f6cfa5baddd5260aeb13 Will be coming forth with 5 and 6. Andrei
Jun 22 2015
prev sibling next sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/21/15 4:47 AM, Dicebot wrote:
 5.
 http://erdani.com/d/phobos-prerelease/std_experimental_allocator_stats_collector.html
 has no overview documentation at all
https://github.com/D-Programming-Language/phobos/commit/ef6de891197db6d497df11c9350781eef38df196 Andrei
Jun 22 2015
prev sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 6/21/15 4:47 AM, Dicebot wrote:
 6. Usage of ternary is not always clear / justified. In `IAllocator` it
 is explained and makes sense but there are things like
 http://erdani.com/d/phobos-prerelease/std_experimental_allocator_bitmapped_block.html
 ("Ternary empty() - Returns true iff no memory is currently allocated
 with this allocator"). I am still not sure why it is used there instead
 of simple boolean.
Founds a bunch of issues with the docs indeed; switching the static interface from bool to Ternary (for the sake of unification with IAllocator) was a last-minute thing. https://github.com/D-Programming-Language/phobos/commit/16f5a5631cf1f2eeea3779785c3ea17682b8ff45 Andrei
Jun 22 2015
prev sibling next sibling parent reply "Dicebot" <public dicebot.lv> writes:
1 day remaining
Jun 25 2015
parent reply "Dicebot" <public dicebot.lv> writes:
On Thursday, 25 June 2015 at 13:36:31 UTC, Dicebot wrote:
 1 day remaining
FYI: considering http://forum.dlang.org/thread/mmhjqe$2mud$1 digitalmars.com there will be some delay between finishing the review period and starting voting process. It is ok to use that time for any additional comments :P
Jun 26 2015
parent "jmh530" <john.michael.hall gmail.com> writes:
On Friday, 26 June 2015 at 13:35:34 UTC, Dicebot wrote:
It is ok to use that time for any
 additional comments :P
I would second the suggestion for improvement in the documentation. The overview linked to in the original post is written assuming advanced programming knowledge. My understanding of allocation is pretty much limited to something could be on stack or heap (I had to Google what an allocator was). So I would say it could be improved by adding some explanation of the basics and why to use allocators instead of new or other techniques. I had seen this when it was originally posted two weeks ago, but I came back around to read it again today. What brought me to look it over again was that I had come across the std.container.array package and I didn't understand how it was different from the built-in array (I still don't have a very good understanding there, but a little better). Anyway, as I was searching through the Learn forum, I came across a post from 2011 saying that std.container might change in the future based on Andrei's work on allocators. I take it this is the work it was referring to (especially after looking at the wikipedia page for allocators). With that I think I only have two questions. Am I correct that the makeArray function is for allocating a dynamic array and not an std.container Array? Are there plans to refactor std.container with std.allocator in mind?
Jun 29 2015
prev sibling next sibling parent reply "Alex Parrill" <initrd.gz gmail.com> writes:
The Windows MMap allocator only keeps one HANDLE around, and 
creates a new one on each `allocate`. Thus, `deallocate` closes 
the latest handle, regardless of what it was actually passed, so 
it leaks.

If I'm reading the docs for `CreateFileMapping` right, you should 
be able to close the handle after calling `MapViewOfFile`; the 
internal data will persist until you unmap the memory region.
Jun 26 2015
parent reply Dmitry Olshansky <dmitry.olsh gmail.com> writes:
On 26-Jun-2015 17:51, Alex Parrill wrote:
 The Windows MMap allocator only keeps one HANDLE around, and creates a
 new one on each `allocate`. Thus, `deallocate` closes the latest handle,
 regardless of what it was actually passed, so it leaks.
Actually I don't see why Windows couldnt' just use VirtualAlloc w/o messing with files.
 If I'm reading the docs for `CreateFileMapping` right, you should be
 able to close the handle after calling `MapViewOfFile`; the internal
 data will persist until you unmap the memory region.
IIRC no you can't. I'd need to double check that though. -- Dmitry Olshansky
Jun 26 2015
parent reply "Alex Parrill" <initrd.gz gmail.com> writes:
On Friday, 26 June 2015 at 14:56:21 UTC, Dmitry Olshansky wrote:
 On 26-Jun-2015 17:51, Alex Parrill wrote:
 The Windows MMap allocator only keeps one HANDLE around, and 
 creates a
 new one on each `allocate`. Thus, `deallocate` closes the 
 latest handle,
 regardless of what it was actually passed, so it leaks.
Actually I don't see why Windows couldnt' just use VirtualAlloc w/o messing with files.
Yea, VirtualAlloc seems like a better fit. (I don't actually know the windows API that well)
 If I'm reading the docs for `CreateFileMapping` right, you 
 should be
 able to close the handle after calling `MapViewOfFile`; the 
 internal
 data will persist until you unmap the memory region.
IIRC no you can't. I'd need to double check that though.
Here's the paragraph I'm reading: Mapped views of a file mapping object maintain internal references to the object, and a file mapping object does not close until all references to it are released. Therefore, to fully close a file mapping object, an application must unmap all mapped views of the file mapping object by calling UnmapViewOfFile and close the file mapping object handle by calling CloseHandle. These functions can be called in any order.
Jun 26 2015
parent reply "Baz" <bb.temp gmx.com> writes:
On Friday, 26 June 2015 at 15:23:25 UTC, Alex Parrill wrote:
 On Friday, 26 June 2015 at 14:56:21 UTC, Dmitry Olshansky wrote:
 [...]
Yea, VirtualAlloc seems like a better fit. (I don't actually know the windows API that well)
 [...]
Here's the paragraph I'm reading: Mapped views of a file mapping object maintain internal references to the object, and a file mapping object does not close until all references to it are released. Therefore, to fully close a file mapping object, an application must unmap all mapped views of the file mapping object by calling UnmapViewOfFile and close the file mapping object handle by calling CloseHandle. These functions can be called in any order.
https://github.com/andralex/phobos/pull/17
Jun 29 2015
parent "Baz" <bb.temp gmx.com> writes:
On Tuesday, 30 June 2015 at 06:07:14 UTC, Baz wrote:
 On Friday, 26 June 2015 at 15:23:25 UTC, Alex Parrill wrote:
 On Friday, 26 June 2015 at 14:56:21 UTC, Dmitry Olshansky 
 wrote:
 [...]
Yea, VirtualAlloc seems like a better fit. (I don't actually know the windows API that well)
 [...]
Here's the paragraph I'm reading: Mapped views of a file mapping object maintain internal references to the object, and a file mapping object does not close until all references to it are released. Therefore, to fully close a file mapping object, an application must unmap all mapped views of the file mapping object by calling UnmapViewOfFile and close the file mapping object handle by calling CloseHandle. These functions can be called in any order.
https://github.com/andralex/phobos/pull/17
By the way, the ddoc comment for mmap needs to be updated (not posix only). I don't know what to write because initially i just wanted to build allocator under win and proposed to the mmap allocator for win without thinking more... currently under win the only difference between a simple malloc is that the allocation happen on commitment...previously it was really an anonymous memory mapped file...).
Jun 29 2015
prev sibling next sibling parent "Martin Nowak" <code dawg.eu> writes:
On Friday, 12 June 2015 at 11:06:43 UTC, Dicebot wrote:
 The legendary allocator package by Andrei Alexandrescu has 
 arrived at your doorsteps and kindly asks to let it into Phobos
Sorry for being late, I wanted to restate an idea that could be crucial for optimizations. https://github.com/D-Programming-Language/druntime/pull/1183#issuecomment-77065554 This reminds me of the fact, that the IAllocator interface (std.allocator) should also have a strongly pure allocate function. This might allow the compiler to optimize allocations even with a dynamic dispatch allocator, because it assumes that an allocation doesn't have a side-effect and always returns "fresh" unaliased memory. Chandler Carruth was talking about this problem at the last CppCon. https://www.youtube.com/watch?v=fHNmRkzxHWs&t=3950 https://www.youtube.com/watch?v=fHNmRkzxHWs&t=4037 Now putting pure onto the allocate function of IAllocator would be a very harsh constraint for any implementation. So maybe we could employ a compiler hack, marking any IAllocator.allocate implementation as pure. This optimization is really important, b/c it allows the compiler to ellide allocations when it can compute something using a temporary buffer. It'd probably require a magic free as well, so that the compiler knows the lifetime and sees that data isn't escaped.
Jul 10 2015
prev sibling parent reply Jacob Carlborg <doob me.com> writes:
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 just looked at Andrei's dconf talk and started to look at the source code how this "hasMember" is used. To me it looks like in most cases the optional methods, like "deallocate", "owns", "expand" and so on, could instead be required methods. Allocators that don't support these methods would need to have dummy implementations. But at the same time it would be easier for the user of the allocators, doesn't need to use all these "static if". The current way of using "hasMember" and "static if" seems to be a more complicated design. -- /Jacob Carlborg
Jul 12 2015
next sibling parent reply Dmitry Olshansky <dmitry.olsh gmail.com> writes:
On 12-Jul-2015 18:08, Jacob Carlborg wrote:
 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 just looked at Andrei's dconf talk and started to look at the source code how this "hasMember" is used. To me it looks like in most cases the optional methods, like "deallocate", "owns", "expand" and so on, could instead be required methods. Allocators that don't support these methods would need to have dummy implementations. But at the same time it would be easier for the user of the allocators, doesn't need to use all these "static if".
But then a Parent allocator would like to know if the child actually doesn't support reallocate vs say failing to reallocate. In the former case a lot of extra logic is simply removed at compile-time. -- Dmitry Olshansky
Jul 12 2015
parent Jacob Carlborg <doob me.com> writes:
On 2015-07-12 18:10, Dmitry Olshansky wrote:

 But then a Parent allocator would like to know if the child actually
 doesn't support reallocate vs say failing to reallocate. In the former
 case a lot of extra logic is simply removed at compile-time.
Perhaps this is a simplified view, but most cases there were no difference between failing to reallocate or does not support reallocate. "deallocate", for example, was used like this: static if (hasMember!("deallocate")) deallocate(); No "else", no action was taken when it didn't support "deallocate". I haven't look through the whole source code. -- /Jacob Carlborg
Jul 14 2015
prev sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 7/12/15 11:08 AM, Jacob Carlborg wrote:
 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 just looked at Andrei's dconf talk and started to look at the source code how this "hasMember" is used. To me it looks like in most cases the optional methods, like "deallocate", "owns", "expand" and so on, could instead be required methods. Allocators that don't support these methods would need to have dummy implementations. But at the same time it would be easier for the user of the allocators, doesn't need to use all these "static if".
That design would have been possible, e.g. have deallocate return false, owns return Ternary.unknown, expand return false, alignedAllocate return null etc. I see the following issues with that design: 1. Many incorrect or grossly unfit allocators can be statically defined, for example a FallbackAllocator putting in the front an allocator that has a dummy implementation of owns. Unittesting could be expected to reasonably get rid of most. But there are many byzantine failure modes that are likely to escape even thorough unittesting. Consider, for example, building a Segregator out of several allocators, most of which support alignedAllocate, but some that don't (i.e. return null). During testing, if only the supported size ranges are tested, it all works. In production, if the wrong size is asked for in alignedAllocate, the allocator will return null as if the system ran out of memory. 2. Efficiency becomes increasingly tenuous. Simple cases of calls to do-nothing functions can be reasonably expected to be handled by the optimizer, but e.g. AllocatorList does significant work in owns, deallocate, deallocateAll, etc. - all under the assumption that the parent does implement the expected functionality. Emery Berger told me this was a constant source of concern with HeapLayers; he'd need to look at disassembly and tweak code in various ways to obtain the desired inlining, which when absent would cause dramatic slowdowns. Compiler technology has improved since that work, but also Heap Building Blocks go quite a longer distance than HeapLayers. With the DbI approach, the performance profile of a composite allocator is immediate and obvious. 3. Layout decisions cannot be made with dummy implementations. Currently there are no layout decisions in std.allocator that depend on presence of members, but ScopedAllocator came very closed to such. (There are many layout decisions that depend on instance size.) Going with a dynamic approach would preclude data layout decisions. Andrei
Jul 13 2015
parent reply Jacob Carlborg <doob me.com> writes:
On 2015-07-13 20:52, Andrei Alexandrescu wrote:

 That design would have been possible, e.g. have deallocate return false,
 owns return Ternary.unknown, expand return false, alignedAllocate return
 null etc.
Perhaps it's a simplified view, but for all examples in the talk it seems it would work without changing the API. The branch which was executed for "does not support this operation" was the same for "this operation failed". Example: static if (hasMember!("deallocate")) deallocate(); There was no "else", no action when "deallocate" was not supported. Or: static if (hasMember!("reallocate")) return reallocate(); else return false; Of course I haven't look through the whole source code.
 I see the following issues with that design:

 1. Many incorrect or grossly unfit allocators can be statically defined,
 for example a FallbackAllocator putting in the front an allocator that
 has a dummy implementation of owns. Unittesting could be expected to
 reasonably get rid of most.

 But there are many byzantine failure modes that are likely to escape
 even thorough unittesting. Consider, for example, building a Segregator
 out of several allocators, most of which support alignedAllocate, but
 some that don't (i.e. return null). During testing, if only the
 supported size ranges are tested, it all works. In production, if the
 wrong size is asked for in alignedAllocate, the allocator will return
 null as if the system ran out of memory.

 2. Efficiency becomes increasingly tenuous. Simple cases of calls to
 do-nothing functions can be reasonably expected to be handled by the
 optimizer, but e.g. AllocatorList does significant work in owns,
 deallocate, deallocateAll, etc. - all under the assumption that the
 parent does implement the expected functionality.

 Emery Berger told me this was a constant source of concern with
 HeapLayers; he'd need to look at disassembly and tweak code in various
 ways to obtain the desired inlining, which when absent would cause
 dramatic slowdowns. Compiler technology has improved since that work,
 but also Heap Building Blocks go quite a longer distance than HeapLayers.

 With the DbI approach, the performance profile of a composite allocator
 is immediate and obvious.
Yeah, my suggestion assumes the compiler can optimize away all these dummy functions. -- /Jacob Carlborg
Jul 14 2015
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 7/14/15 7:35 AM, Jacob Carlborg wrote:
 On 2015-07-13 20:52, Andrei Alexandrescu wrote:

 That design would have been possible, e.g. have deallocate return false,
 owns return Ternary.unknown, expand return false, alignedAllocate return
 null etc.
Perhaps it's a simplified view, but for all examples in the talk it seems it would work without changing the API. The branch which was executed for "does not support this operation" was the same for "this operation failed". Example: static if (hasMember!("deallocate")) deallocate(); There was no "else", no action when "deallocate" was not supported. Or: static if (hasMember!("reallocate")) return reallocate(); else return false;
I explained matters at length in the post you're replying to, and from what I can tell this reply is a reiteration of your same point. I don't know what to add - you may want to pay some more mind to that post.
 Of course I haven't look through the whole source code.
One easy way to take a look at things is to clone the branch and then: git grep --context=5 hasMember or however many lines you need.
 Yeah, my suggestion assumes the compiler can optimize away all these
 dummy functions.
There were two other considerations. Andrei
Jul 14 2015
parent reply "Tofu Ninja" <emmons0 purdue.edu> writes:
Something I thought about today, if a class or struct is 
allocated by an allocator(say a malloc based allocator) and one 
of its fields is populated with a reference to GCed memory, will 
the GC know to scan the allocator memory to keep that GCed memory 
alive?
Aug 13 2015
parent reply "rsw0x" <anonymous anonymous.com> writes:
On Thursday, 13 August 2015 at 20:56:36 UTC, Tofu Ninja wrote:
 Something I thought about today, if a class or struct is 
 allocated by an allocator(say a malloc based allocator) and one 
 of its fields is populated with a reference to GCed memory, 
 will the GC know to scan the allocator memory to keep that GCed 
 memory alive?
http://dlang.org/phobos/core_memory.html#.GC.addRange
Aug 13 2015
parent "Tofu Ninja" <emmons0 purdue.edu> writes:
On Thursday, 13 August 2015 at 21:05:28 UTC, rsw0x wrote:
 On Thursday, 13 August 2015 at 20:56:36 UTC, Tofu Ninja wrote:
 Something I thought about today, if a class or struct is 
 allocated by an allocator(say a malloc based allocator) and 
 one of its fields is populated with a reference to GCed 
 memory, will the GC know to scan the allocator memory to keep 
 that GCed memory alive?
http://dlang.org/phobos/core_memory.html#.GC.addRange
Yeah I know about that, I was just asking if all the allocators would be marked to be scanned by default. Or if they might try to only mark as needing to be scanned if they contain types that could contain references. The information of which types have internal references seems to be available at least to the GC. Theoretically an allocator could use that information to mark its allocations as needing to be scanned as well. Or it could be a wrapper allocator that simply marks things as needing to be scanned when they need to be, that way it would be an opt-in type deal. just some thoughts...
Aug 13 2015