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.  



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




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?
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?





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