www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Phobos for Review: std.buffer.scopebuffer

reply Walter Bright <newshound2 digitalmars.com> writes:
https://github.com/D-Programming-Language/phobos/pull/1911

This adds a package std.buffer, and the first entry in that package, 
std.buffer.scopebuffer.

ScopeBuffer is an OutputRange that sits on the stack, and overflows to 
malloc/free. It's designed to help in eliminating gc allocation by lower level 
functions that return buffers, such as std.path.buildPath(). With some
judicious 
user tuning of the initial stack size, it can virtually eliminate storage 
allocation.

Using it is  system, but the user of ScopeBuffer can be  trusted.

An output range like this is a precursor to eliminating the excessive gc use by 
functions such as buildPath().
Feb 07 2014
next sibling parent reply "Brad Anderson" <eco gnuk.net> writes:
On Friday, 7 February 2014 at 08:24:39 UTC, Walter Bright wrote:

 https://github.com/D-Programming-Language/phobos/pull/1911

 This adds a package std.buffer, and the first entry in that 
 package, std.buffer.scopebuffer.


About to go to bed so just a couple of quick thoughts.

Why not just stick the stack array in ScopeBuffer itself (the
size could be a template parameter)? Escaping a ScopeBuffer is
rarely safe when it takes an external static array but if it
packed its own static array you could safely return it safely.

Also, if it did embed the static array in the ScopeBuffer I
wonder if it could just use the pointer as the buffer if the
length is less than size_t (short string optimization).

(Just thinking aloud, I wonder if std.allocators could be used to
implement this more generically (hypothetically: alias
ScopeBuffer =
CustomAllocOutputRange!(StaticWithMallocFallbackAllocator!10)))

It's a welcome addition and will go a long way toward cutting
down GC allocations once Phobos is output-range-ified. This 
technique is probably the first thing I'd reach for when choosing 
an output range.
Feb 07 2014
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 2/7/2014 12:46 AM, Brad Anderson wrote:

 On Friday, 7 February 2014 at 08:24:39 UTC, Walter Bright wrote:

 https://github.com/D-Programming-Language/phobos/pull/1911

 This adds a package std.buffer, and the first entry in that package,
 std.buffer.scopebuffer.


 About to go to bed so just a couple of quick thoughts.

 Why not just stick the stack array in ScopeBuffer itself (the
 size could be a template parameter)?


1. It's set up to fit into two registers on 64 bit code. This means it can be 
passed/returned from functions in registers. When I used this in my own code, 
high speed was the top priority, and this made a difference.

2. It needs to avoid the default initialization of the array, because it's both
unnecessary and it kills the speed. Currently, this
cannot be avoided for part of a struct.

3. I wanted it to be usable for any block of memory the user wished to dedicate 
to be a buffer.

4. Having an internal reference like that would mean a postblit is required for 
copying/moving the range, another source of speed degradation.


The design of ScopeBuffer is based on a fair amount of trial and error on my 
part trying to wring as much speed as possible out of it.



 Also, if it did embed the static array in the ScopeBuffer I
 wonder if it could just use the pointer as the buffer if the
 length is less than size_t (short string optimization).


I almost never needed a buffer that small, and the extra tests for it killed 
performance (I had tried it at one point).



 (Just thinking aloud, I wonder if std.allocators could be used to
 implement this more generically (hypothetically: alias
 ScopeBuffer =
 CustomAllocOutputRange!(StaticWithMallocFallbackAllocator!10)))


The custom allocator design is a long way from being realized. I don't know if 
it would help or not.



 It's a welcome addition and will go a long way toward cutting
 down GC allocations once Phobos is output-range-ified. This technique is
 probably the first thing I'd reach for when choosing an output range.


Once the buffer package is accepted, I want to add others I've been using like 
circular buffers (the one from the compressor proposal), fixed buffers, and
even 
move std.outbuffer to it. I also want it to be one buffer design per module,
not 
the usual grab-bag modules.
Feb 07 2014
next sibling parent reply dennis luehring <dl.soluz gmx.net> writes:
Am 07.02.2014 10:13, schrieb Walter Bright:

 1. It's set up to fit into two registers on 64 bit code. This means it can be
 passed/returned from functions in registers. When I used this in my own code,
 high speed was the top priority, and this made a difference.


did you also test the codegen of ldc and gdc - or is this optimization
only "working" for dmd?
Feb 07 2014
parent Walter Bright <newshound2 digitalmars.com> writes:
On 2/7/2014 2:27 AM, dennis luehring wrote:

 Am 07.02.2014 10:13, schrieb Walter Bright:

 1. It's set up to fit into two registers on 64 bit code. This means it can be
 passed/returned from functions in registers. When I used this in my own code,
 high speed was the top priority, and this made a difference.


 did you also test the codegen of ldc and gdc - or is this optimization
 only "working" for dmd?


It's part of the C ABI.
Feb 07 2014
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 2/7/2014 1:13 AM, Walter Bright wrote:

 On 2/7/2014 12:46 AM, Brad Anderson wrote:

 Why not just stick the stack array in ScopeBuffer itself (the
 size could be a template parameter)?


 1. It's set up to fit into two registers on 64 bit code. This means it can be
 passed/returned from functions in registers. When I used this in my own code,
 high speed was the top priority, and this made a difference.

 2. It needs to avoid the default initialization of the array, because it's both
 unnecessary and it kills the speed. Currently, this
 cannot be avoided for part of a struct.

 3. I wanted it to be usable for any block of memory the user wished to dedicate
 to be a buffer.

 4. Having an internal reference like that would mean a postblit is required for
 copying/moving the range, another source of speed degradation.


5. Every instantiation that only differs by the buffer size would generate a 
separate set of code. Having the buffer size passed into the constructor means 
only one instance is generated per type.
Feb 07 2014
parent reply "Brad Anderson" <eco gnuk.net> writes:
On Friday, 7 February 2014 at 22:12:29 UTC, Walter Bright wrote:

 5. Every instantiation that only differs by the buffer size 
 would generate a separate set of code. Having the buffer size 
 passed into the constructor means only one instance is 
 generated per type.


There's always alloca :)
Feb 07 2014
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 2/7/2014 2:14 PM, Brad Anderson wrote:

 On Friday, 7 February 2014 at 22:12:29 UTC, Walter Bright wrote:

 5. Every instantiation that only differs by the buffer size would generate a
 separate set of code. Having the buffer size passed into the constructor means
 only one instance is generated per type.


 There's always alloca :)


alloca() cannot be used to allocate stack data in a function enclosing the 
current one.
Feb 07 2014
parent reply "Brad Anderson" <eco gnuk.net> writes:
On Friday, 7 February 2014 at 23:10:50 UTC, Walter Bright wrote:

 On 2/7/2014 2:14 PM, Brad Anderson wrote:

 There's always alloca :)


 alloca() cannot be used to allocate stack data in a function 
 enclosing the current one.


Oh, right. Forgot about that.
Feb 07 2014
next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 2/7/14, 3:11 PM, Brad Anderson wrote:

 On Friday, 7 February 2014 at 23:10:50 UTC, Walter Bright wrote:

 On 2/7/2014 2:14 PM, Brad Anderson wrote:

 There's always alloca :)


 alloca() cannot be used to allocate stack data in a function enclosing
 the current one.


 Oh, right. Forgot about that.


You can with a default parameter...

void fun(void* sneaky = alloca(42));

will allocate memory on fun caller's frame and make it available to fun.

I've known this for a while and am not sure whether it's an awesome 
idiom of the spawn of devil.


Andrei
Feb 07 2014
next sibling parent Marco Leise <Marco.Leise gmx.de> writes:
Am Fri, 07 Feb 2014 16:59:33 -0800
schrieb Andrei Alexandrescu <SeeWebsiteForEmail erdani.org>:


 On 2/7/14, 3:11 PM, Brad Anderson wrote:

 On Friday, 7 February 2014 at 23:10:50 UTC, Walter Bright wrote:

 On 2/7/2014 2:14 PM, Brad Anderson wrote:

 There's always alloca :)


 alloca() cannot be used to allocate stack data in a function enclosing
 the current one.


 Oh, right. Forgot about that.


 
 You can with a default parameter...
 
 void fun(void* sneaky = alloca(42));
 
 will allocate memory on fun caller's frame and make it available to fun.
 
 I've known this for a while and am not sure whether it's an awesome 
 idiom of the spawn of devil.
 
 
 Andrei


I only recently discovered this method, but put it to good use
as well:
https://github.com/mleise/fast/blob/master/source/fast/cstring.d#L134

-- 
Marco
Feb 07 2014
prev sibling next sibling parent reply "Lars T. Kyllingstad" <public kyllingen.net> writes:
On Saturday, 8 February 2014 at 00:59:35 UTC, Andrei Alexandrescu 
wrote:

 void fun(void* sneaky = alloca(42));

 will allocate memory on fun caller's frame and make it 
 available to fun.

 I've known this for a while and am not sure whether it's an 
 awesome idiom of the spawn of devil.


Quoting the GNU alloca() man page:

"On many systems alloca() cannot be used inside the list of 
arguments of a function call, because the stack space reserved by 
alloca() would appear on the stack in the middle of the space for 
the function arguments."

So... devil spawn, then?
Feb 07 2014
parent "Ola Fosheim =?UTF-8?B?R3LDuHN0YWQi?= writes:
On Saturday, 8 February 2014 at 07:43:06 UTC, Lars T. Kyllingstad 
wrote:

 "On many systems alloca() cannot be used inside the list of 
 arguments of a function call, because the stack space reserved 
 by alloca() would appear on the stack in the middle of the 
 space for the function arguments."

 So... devil spawn, then?


alloca() is compiler dependent. LLVM has experimental support for 
alloca in parameters using a dedicated specification. This is the 
downside of depending on backends developed for C. For instance 
if the backend emits code that is relative to the end of the 
stack rather than the beginning of the stack frame you have to 
move down all locals whenever you call alloca():

{
BUFFER64K buffer;
auto dynarr = alloca(smallsize); // triggers a copy of 64KiB and 
fucks up all pointers to buffer
}

If you can rely on indexing relative to the beginning  of the 
stack frame you just have to change the stack pointer. If you 
insist on returns of a single dynamic sized temporary being at 
the end of the stack upon return (+whatever RET eats) you can 
just move your own stack frame, I believe.

Thus not wasting a lot of space of the caller stack frame and 
return the length, then the caller can adjust it's own stack 
pointer by adding the offset+length. And prepending to that 
dynamic array is cheap in simple function bodies, just push 
values onto the stack.

But the IR/backend must support it. C-semantics are viral… :-(
Feb 08 2014
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 2/7/2014 4:59 PM, Andrei Alexandrescu wrote:

 On 2/7/14, 3:11 PM, Brad Anderson wrote:

 On Friday, 7 February 2014 at 23:10:50 UTC, Walter Bright wrote:

 On 2/7/2014 2:14 PM, Brad Anderson wrote:

 There's always alloca :)


 alloca() cannot be used to allocate stack data in a function enclosing
 the current one.


 Oh, right. Forgot about that.


 You can with a default parameter...

 void fun(void* sneaky = alloca(42));

 will allocate memory on fun caller's frame and make it available to fun.


I think there's a bit of a problem in how the user specifies the 42?
Feb 08 2014
parent reply Marco Leise <Marco.Leise gmx.de> writes:
Am Sat, 08 Feb 2014 02:13:18 -0800
schrieb Walter Bright <newshound2 digitalmars.com>:


 On 2/7/2014 4:59 PM, Andrei Alexandrescu wrote:

 On 2/7/14, 3:11 PM, Brad Anderson wrote:

 On Friday, 7 February 2014 at 23:10:50 UTC, Walter Bright wrote:

 On 2/7/2014 2:14 PM, Brad Anderson wrote:

 There's always alloca :)


 alloca() cannot be used to allocate stack data in a function enclosing
 the current one.


 Oh, right. Forgot about that.


 You can with a default parameter...

 void fun(void* sneaky = alloca(42));

 will allocate memory on fun caller's frame and make it available to fun.


 
 I think there's a bit of a problem in how the user specifies the 42?


It depends on what syntax you expect. I solved it this way:

void fun(alias length)(void* sneaky = alloca(length));

void foo()
{
    size_t myLength = 42;
    fun!myLength();
}

-- 
Marco
Feb 08 2014
next sibling parent reply "Jakob Ovrum" <jakobovrum gmail.com> writes:
On Sunday, 9 February 2014 at 04:14:05 UTC, Marco Leise wrote:

 It depends on what syntax you expect. I solved it this way:

 void fun(alias length)(void* sneaky = alloca(length));

 void foo()
 {
     size_t myLength = 42;
     fun!myLength();
 }


It defeats the point of alloca if the chunk size has to be known 
at compile-time. You could just use a void-initialized 
fixed-length array at that point.
Feb 09 2014
parent reply Timon Gehr <timon.gehr gmx.ch> writes:
On 02/09/2014 01:12 PM, Jakob Ovrum wrote:

 On Sunday, 9 February 2014 at 04:14:05 UTC, Marco Leise wrote:

 It depends on what syntax you expect. I solved it this way:

 void fun(alias length)(void* sneaky = alloca(length));

 void foo()
 {
     size_t myLength = 42;
     fun!myLength();
 }


 It defeats the point of alloca if the chunk size has to be known at
 compile-time. ...


Why would it need to be known at compile time? 42 is just an example 
here; any other expression will do.
Feb 09 2014
parent "Jakob Ovrum" <jakobovrum gmail.com> writes:
On Sunday, 9 February 2014 at 12:25:51 UTC, Timon Gehr wrote:

 On 02/09/2014 01:12 PM, Jakob Ovrum wrote:

 On Sunday, 9 February 2014 at 04:14:05 UTC, Marco Leise wrote:

 It depends on what syntax you expect. I solved it this way:

 void fun(alias length)(void* sneaky = alloca(length));

 void foo()
 {
    size_t myLength = 42;
    fun!myLength();
 }


 It defeats the point of alloca if the chunk size has to be 
 known at
 compile-time. ...


 Why would it need to be known at compile time? 42 is just an 
 example here; any other expression will do.


Nevermind, didn't see the alias.
Feb 09 2014
prev sibling parent reply "Dicebot" <public dicebot.lv> writes:
On Sunday, 9 February 2014 at 04:14:05 UTC, Marco Leise wrote:

 It depends on what syntax you expect. I solved it this way:

 void fun(alias length)(void* sneaky = alloca(length));

 void foo()
 {
     size_t myLength = 42;
     fun!myLength();
 }


Huge template bloat in current frontend state - new instance of 
`fun` for every single new function it is used in.
Feb 09 2014
next sibling parent reply "Jakob Ovrum" <jakobovrum gmail.com> writes:
On Sunday, 9 February 2014 at 12:36:38 UTC, Dicebot wrote:

 On Sunday, 9 February 2014 at 04:14:05 UTC, Marco Leise wrote:

 It depends on what syntax you expect. I solved it this way:

 void fun(alias length)(void* sneaky = alloca(length));

 void foo()
 {
    size_t myLength = 42;
    fun!myLength();
 }


 Huge template bloat in current frontend state - new instance of 
 `fun` for every single new function it is used in.


The implementation of `fun` can just forward `sneaky` onto 
another function. Cue some simple inlining and there is no bloat 
(at runtime, anyhow).
Feb 09 2014
parent reply "Dicebot" <public dicebot.lv> writes:
On Sunday, 9 February 2014 at 12:57:32 UTC, Jakob Ovrum wrote:

 The implementation of `fun` can just forward `sneaky` onto 
 another function. Cue some simple inlining and there is no 
 bloat (at runtime, anyhow).


I am not speaking about what "sufficiently clever compiler" can 
do but what current frontend does. New aliased identifier == new 
instance. Always.
Feb 09 2014
parent reply "Jakob Ovrum" <jakobovrum gmail.com> writes:
On Sunday, 9 February 2014 at 13:08:45 UTC, Dicebot wrote:

 On Sunday, 9 February 2014 at 12:57:32 UTC, Jakob Ovrum wrote:

 The implementation of `fun` can just forward `sneaky` onto 
 another function. Cue some simple inlining and there is no 
 bloat (at runtime, anyhow).


 I am not speaking about what "sufficiently clever compiler" can 
 do but what current frontend does. New aliased identifier == 
 new instance. Always.


Yes, but a new template instance is not necessarily a problem at 
runtime or for the executable size. The contents of the template 
instance is all that matters. When the content is a tiny 
inlinable function, there is no bloat.
Feb 09 2014
parent reply "Dicebot" <public dicebot.lv> writes:
On Sunday, 9 February 2014 at 13:11:57 UTC, Jakob Ovrum wrote:

 Yes, but a new template instance is not necessarily a problem 
 at runtime or for the executable size. The contents of the 
 template instance is all that matters. When the content is a 
 tiny inlinable function, there is no bloat.


I have investiagted relevant parts of DMD for months. It simply 
does not work that way and changing it would require huge 
frontend refactoring. Inlining does not affect bloat in DMD at 
all. Every instance that is ever mentioned in code will always 
make its way to the target binary. You can trivially check it.
Feb 09 2014
parent reply "Jakob Ovrum" <jakobovrum gmail.com> writes:
On Sunday, 9 February 2014 at 13:14:00 UTC, Dicebot wrote:

 On Sunday, 9 February 2014 at 13:11:57 UTC, Jakob Ovrum wrote:

 Yes, but a new template instance is not necessarily a problem 
 at runtime or for the executable size. The contents of the 
 template instance is all that matters. When the content is a 
 tiny inlinable function, there is no bloat.


 I have investiagted relevant parts of DMD for months. It simply 
 does not work that way and changing it would require huge 
 frontend refactoring. Inlining does not affect bloat in DMD at 
 all. Every instance that is ever mentioned in code will always 
 make its way to the target binary. You can trivially check it.


It is the linker's job to purge unused code.
Feb 09 2014
parent reply "Dicebot" <public dicebot.lv> writes:
On Sunday, 9 February 2014 at 13:19:40 UTC, Jakob Ovrum wrote:

 I have investiagted relevant parts of DMD for months. It 
 simply does not work that way and changing it would require 
 huge frontend refactoring. Inlining does not affect bloat in 
 DMD at all. Every instance that is ever mentioned in code will 
 always make its way to the target binary. You can trivially 
 check it.


 It is the linker's job to purge unused code.


Please show me the one who is capable of doing it. ld's 
--gc-sections currently breaks binaries and it is the only one I 
am aware about.
Feb 09 2014
parent Walter Bright <newshound2 digitalmars.com> writes:
On 2/9/2014 5:48 AM, Dicebot wrote:

 On Sunday, 9 February 2014 at 13:19:40 UTC, Jakob Ovrum wrote:

 I have investiagted relevant parts of DMD for months. It simply does not work
 that way and changing it would require huge frontend refactoring. Inlining
 does not affect bloat in DMD at all. Every instance that is ever mentioned in
 code will always make its way to the target binary. You can trivially check it.


 It is the linker's job to purge unused code.


 Please show me the one who is capable of doing it. ld's --gc-sections currently
 breaks binaries and it is the only one I am aware about.


You're right. Unfortunately, D has to use existing linkers. That means we
cannot 
build the compiler to require a possible, desirable, but non-existent linking 
technology.
Feb 09 2014
prev sibling parent Marco Leise <Marco.Leise gmx.de> writes:
Am Sun, 09 Feb 2014 12:36:36 +0000
schrieb "Dicebot" <public dicebot.lv>:


 On Sunday, 9 February 2014 at 04:14:05 UTC, Marco Leise wrote:

 It depends on what syntax you expect. I solved it this way:

 void fun(alias length)(void* sneaky = alloca(length));

 void foo()
 {
     size_t myLength = 42;
     fun!myLength();
 }


 
 Huge template bloat in current frontend state - new instance of 
 `fun` for every single new function it is used in.


Thanks for the hint! I'll keep that in mind when the function
becomes larger.

-- 
Marco
Feb 16 2014
prev sibling parent "Ola Fosheim =?UTF-8?B?R3LDuHN0YWQi?= writes:
On Friday, 7 February 2014 at 23:11:42 UTC, Brad Anderson wrote:

 On Friday, 7 February 2014 at 23:10:50 UTC, Walter Bright wrote:

 On 2/7/2014 2:14 PM, Brad Anderson wrote:

 There's always alloca :)


 alloca() cannot be used to allocate stack data in a function 
 enclosing the current one.


 Oh, right. Forgot about that.


(Of course you can. Modify the parent's stack frame and return by 
a JMP. ;-)
Feb 07 2014
prev sibling next sibling parent reply "Lars T. Kyllingstad" <public kyllingen.net> writes:
On Friday, 7 February 2014 at 08:24:39 UTC, Walter Bright wrote:

 [...]

 An output range like this is a precursor to eliminating the 
 excessive gc use by functions such as buildPath().


Somewhat OT, but I think you should pick some other function than 
buildPath() for your examples.  It *used* to allocate east and 
west, yes, but that was a holdover from the old std.path.join().  
I made some improvements to it about half a year ago, and now it 
virtually never allocates more than once(*).

Note, this is not an argument against ScopeBuffer, which may well 
be very useful.  (I haven't looked properly at it yet.)

Lars


(*) Specifically, it will allocate more than once iff the list of 
path segments is an input range, and not a forward range, and the 
resulting path is longer than 255 characters.
Feb 07 2014
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 2/7/2014 1:13 AM, Lars T. Kyllingstad wrote:

 On Friday, 7 February 2014 at 08:24:39 UTC, Walter Bright wrote:

 An output range like this is a precursor to eliminating the excessive gc use
 by functions such as buildPath().


 Somewhat OT, but I think you should pick some other function than buildPath()
 for your examples.  It *used* to allocate east and west, yes, but that was a
 holdover from the old std.path.join(). I made some improvements to it about
half
 a year ago, and now it virtually never allocates more than once(*).


I picked on buildPath() for a reason. The program I was writing did a lot of 
searching for files along a path, and by a lot I mean tens of thousands of
times 
(!).

That one little 'ole allocation was murdering performance and generating vast 
amounts of garbage.

I wound up rewriting it to use ScopeBuffer, problem solved.

(Also, one of the lovely things about ScopeBuffer is it moves the action to the 
stack, which is hot in the cache.)
Feb 07 2014
next sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 2/7/2014 1:38 AM, Walter Bright wrote:

 I picked on buildPath() for a reason.


Sorry :-)

Anyhow, I think buildPath() is a good example of why even an innocuous 
allocation can cause problems.
Feb 07 2014
parent "Lars T. Kyllingstad" <public kyllingen.net> writes:
On Friday, 7 February 2014 at 09:44:07 UTC, Walter Bright wrote:

 On 2/7/2014 1:38 AM, Walter Bright wrote:

 I picked on buildPath() for a reason.


 Sorry :-)


No worries. :)  I'm not being defensive here; I just want to 
understand, and I also don't want newbies to needlessly avoid 
buildPath() because they think it is a performance killer.
Feb 07 2014
prev sibling parent reply "Lars T. Kyllingstad" <public kyllingen.net> writes:
On Friday, 7 February 2014 at 09:38:56 UTC, Walter Bright wrote:

 I picked on buildPath() for a reason. The program I was writing 
 did a lot of searching for files along a path, and by a lot I 
 mean tens of thousands of times (!).

 That one little 'ole allocation was murdering performance and 
 generating vast amounts of garbage.


I don't understand.  Even if your workspace is stack-based or 
malloc-ed, you still need that one GC allocation for the return 
value, no?

Lars
Feb 07 2014
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 2/7/2014 2:02 AM, Lars T. Kyllingstad wrote:

 On Friday, 7 February 2014 at 09:38:56 UTC, Walter Bright wrote:

 I picked on buildPath() for a reason. The program I was writing did a lot of
 searching for files along a path, and by a lot I mean tens of thousands of
 times (!).

 That one little 'ole allocation was murdering performance and generating vast
 amounts of garbage.


 I don't understand.  Even if your workspace is stack-based or malloc-ed, you
 still need that one GC allocation for the return value, no?


If you have a path with 20 entries in it, and don't find the file, you do zero 
allocations instead of 20. If you find the file, that's do one allocation 
instead of 20/2 (on average), and you might be able to avoid that by passing it 
upwards, too :-)
Feb 07 2014
parent reply "Lars T. Kyllingstad" <public kyllingen.net> writes:
On Friday, 7 February 2014 at 10:29:07 UTC, Walter Bright wrote:

 On 2/7/2014 2:02 AM, Lars T. Kyllingstad wrote:

 I don't understand.  Even if your workspace is stack-based or 
 malloc-ed, you
 still need that one GC allocation for the return value, no?


 If you have a path with 20 entries in it, and don't find the 
 file, you do zero allocations instead of 20. If you find the 
 file, that's do one allocation instead of 20/2 (on average), 
 and you might be able to avoid that by passing it upwards, too 
 :-)


Ah, now I understand.  I misunderstood you -- I thought you meant 
that using ScopeBuffer to build the return array inside 
buildPath(), while retaining the function's API, would somehow 
improve its performance greatly.  But what you're saying is that 
you would change the signature as well, to something like this:

   void buildPath(IR, OR)(IR segments, OR result)
       if (isInputRange!IR && isOutputRange!(OR, char));

I fully agree, then, and I see how ScopeBuffer would be extremely 
useful if more of Phobos' functions were written like this.  In 
the specific case of buildPath(), I've actually considered 
letting the user supply their own buffer in the form of an array, 
but this is of course a more general solution.
Feb 07 2014
next sibling parent reply "Adam D. Ruppe" <destructionator gmail.com> writes:
On Friday, 7 February 2014 at 11:23:31 UTC, Lars T. Kyllingstad 
wrote:

 But what you're saying is that you would change the signature 
 as well, to something like this:


See my output range thread too: we might be able to still return 
the slice while taking the OR for result, and also through in a 
default value so things just work without changes.

Some caution would be needed by the caller to understand that if 
you pass a scope buffer as the output range, the returned slice 
is owned by that buffer too... but that's a managable 
restriction, they did pass the buffer after all so it is no 
surprise to them when it gets used!
Feb 07 2014
parent Walter Bright <newshound2 digitalmars.com> writes:
On 2/7/2014 7:57 AM, Adam D. Ruppe wrote:

 Some caution would be needed by the caller to understand that if you pass a
 scope buffer as the output range, the returned slice is owned by that buffer
 too... but that's a managable restriction, they did pass the buffer after all
so
 it is no surprise to them when it gets used!


That's why the [ ] overloads are marked  system.
Feb 07 2014
prev sibling next sibling parent reply Jerry <jlquinn optonline.net> writes:
"Lars T. Kyllingstad" <public kyllingen.net> writes:


 On Friday, 7 February 2014 at 10:29:07 UTC, Walter Bright wrote:
 Ah, now I understand.  I misunderstood you -- I thought you meant that using
 ScopeBuffer to build the return array inside buildPath(), while retaining the
 function's API, would somehow improve its performance greatly.  But what
 you're saying is that you would change the signature as well, to something
 like this:

   void buildPath(IR, OR)(IR segments, OR result)
       if (isInputRange!IR && isOutputRange!(OR, char));

 I fully agree, then, and I see how ScopeBuffer would be extremely useful if
 more of Phobos' functions were written like this.  In the specific case of
 buildPath(), I've actually considered letting the user supply their own buffer
 in the form of an array, but this is of course a more general solution.


I'd suggest reversing the arguments:

  void buildPath(IR, OR)(OR result, IR segments)
      if (isInputRange!IR && isOutputRange!(OR, char));

That way you can use it as:

buffer.buildPath(p1, p2, ...);

It at least opens up chaining possibilities.
Feb 07 2014
parent reply "Brad Anderson" <eco gnuk.net> writes:
On Friday, 7 February 2014 at 21:05:43 UTC, Jerry wrote:

 I'd suggest reversing the arguments:

   void buildPath(IR, OR)(OR result, IR segments)
       if (isInputRange!IR && isOutputRange!(OR, char));

 That way you can use it as:

 buffer.buildPath(p1, p2, ...);

 It at least opens up chaining possibilities.


On the other hand the output buffer last allows stuff like this 
contrived example:

"some/foo/path"
   .splitter("/")
   .buildPath(buffer);

I'm not sure what would be more common and useful.
Feb 07 2014
parent reply "Jakob Ovrum" <jakobovrum gmail.com> writes:
On Friday, 7 February 2014 at 21:24:16 UTC, Brad Anderson wrote:

 On Friday, 7 February 2014 at 21:05:43 UTC, Jerry wrote:

 I'd suggest reversing the arguments:

  void buildPath(IR, OR)(OR result, IR segments)
      if (isInputRange!IR && isOutputRange!(OR, char));

 That way you can use it as:

 buffer.buildPath(p1, p2, ...);

 It at least opens up chaining possibilities.


 On the other hand the output buffer last allows stuff like this 
 contrived example:

 "some/foo/path"
   .splitter("/")
   .buildPath(buffer);

 I'm not sure what would be more common and useful.


Those are two different overloads, so I think we could do both.
Feb 07 2014
parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Friday, February 07, 2014 22:15:11 Jakob Ovrum wrote:

 On Friday, 7 February 2014 at 21:24:16 UTC, Brad Anderson wrote:

 On Friday, 7 February 2014 at 21:05:43 UTC, Jerry wrote:

 I'd suggest reversing the arguments:
  void buildPath(IR, OR)(OR result, IR segments)
  
      if (isInputRange!IR && isOutputRange!(OR, char));
 
 That way you can use it as:
 
 buffer.buildPath(p1, p2, ...);
 
 It at least opens up chaining possibilities.


 
 On the other hand the output buffer last allows stuff like this
 contrived example:
 
 "some/foo/path"
 
   .splitter("/")
   .buildPath(buffer);
 
 I'm not sure what would be more common and useful.


 
 Those are two different overloads, so I think we could do both.


That's assuming that the same type wouldn't match both parameters, which would 
be very easy to do if you end up with any ranges that function as both input 
and output ranges (e.g. dchar[] would match both). I'm not sure how often 
you'd get an ambiguity error in practice, but it seems risky to me - and it 
potentially makes it harder to read code that uses it if you can arbitrarily 
flip the arguments.

- Jonathan M Davis
Feb 07 2014
prev sibling parent reply "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Friday, February 07, 2014 11:23:30 Lars T. Kyllingstad wrote:

 On Friday, 7 February 2014 at 10:29:07 UTC, Walter Bright wrote:

 On 2/7/2014 2:02 AM, Lars T. Kyllingstad wrote:

 I don't understand. Even if your workspace is stack-based or
 malloc-ed, you
 still need that one GC allocation for the return value, no?


 
 If you have a path with 20 entries in it, and don't find the
 file, you do zero allocations instead of 20. If you find the
 file, that's do one allocation instead of 20/2 (on average),
 and you might be able to avoid that by passing it upwards, too
 
 :-)


 
 Ah, now I understand. I misunderstood you -- I thought you meant
 that using ScopeBuffer to build the return array inside
 buildPath(), while retaining the function's API, would somehow
 improve its performance greatly. But what you're saying is that
 you would change the signature as well, to something like this:
 
 void buildPath(IR, OR)(IR segments, OR result)
 if (isInputRange!IR && isOutputRange!(OR, char));
 
 I fully agree, then, and I see how ScopeBuffer would be extremely
 useful if more of Phobos' functions were written like this. In
 the specific case of buildPath(), I've actually considered
 letting the user supply their own buffer in the form of an array,
 but this is of course a more general solution.


We really should be moving to a model where any function that returns a new 
array has on overload which takes an output range and uses that for the output 
instead of returning a newly allocated array. That way, code that doesn't need 
the performance boost can just use the return value like we do now, but more 
performance-critical code can use output ranges and avoid GC allocations and 
the like.

- Jonathan M Davis
Feb 07 2014
parent Paulo Pinto <pjmlp progtools.org> writes:
Am 08.02.2014 02:30, schrieb Jonathan M Davis:

 On Friday, February 07, 2014 11:23:30 Lars T. Kyllingstad wrote:

 On Friday, 7 February 2014 at 10:29:07 UTC, Walter Bright wrote:

 On 2/7/2014 2:02 AM, Lars T. Kyllingstad wrote:

 I don't understand. Even if your workspace is stack-based or
 malloc-ed, you
 still need that one GC allocation for the return value, no?


 If you have a path with 20 entries in it, and don't find the
 file, you do zero allocations instead of 20. If you find the
 file, that's do one allocation instead of 20/2 (on average),
 and you might be able to avoid that by passing it upwards, too

 :-)


 Ah, now I understand. I misunderstood you -- I thought you meant
 that using ScopeBuffer to build the return array inside
 buildPath(), while retaining the function's API, would somehow
 improve its performance greatly. But what you're saying is that
 you would change the signature as well, to something like this:

 void buildPath(IR, OR)(IR segments, OR result)
 if (isInputRange!IR && isOutputRange!(OR, char));

 I fully agree, then, and I see how ScopeBuffer would be extremely
 useful if more of Phobos' functions were written like this. In
 the specific case of buildPath(), I've actually considered
 letting the user supply their own buffer in the form of an array,
 but this is of course a more general solution.


 We really should be moving to a model where any function that returns a new
 array has on overload which takes an output range and uses that for the output
 instead of returning a newly allocated array. That way, code that doesn't need
 the performance boost can just use the return value like we do now, but more
 performance-critical code can use output ranges and avoid GC allocations and
 the like.

 - Jonathan M Davis


There quite a few Java libraries that follow this pattern.

--
Paulo
Feb 07 2014
prev sibling next sibling parent reply "Dicebot" <public dicebot.lv> writes:
Do you want this to included into review queue for formal voting?
Feb 07 2014
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 2/7/2014 3:00 AM, Dicebot wrote:

 Do you want this to included into review queue for formal voting?


Sure.
Feb 07 2014
parent reply "Dicebot" <public dicebot.lv> writes:
On Friday, 7 February 2014 at 11:21:54 UTC, Walter Bright wrote:

 On 2/7/2014 3:00 AM, Dicebot wrote:

 Do you want this to included into review queue for formal 
 voting?


 Sure.


Added to review queue : 
http://wiki.dlang.org/Review/std.buffer.scopebuffer

Stepping up as a review manager and turning this into initial 
review thread. Base discussion period is until Feb 17. After that 
I will make summary of concerns raised for you to either address 
or reject with own comment and make decision if it is ready to go 
through voting.

Inofficial list of Phobos quality guidelines can be found here : 
http://wiki.dlang.org/Phobos_Quality

Good luck.
Feb 08 2014
parent Walter Bright <newshound2 digitalmars.com> writes:
On 2/8/2014 4:50 AM, Dicebot wrote:

 Added to review queue : http://wiki.dlang.org/Review/std.buffer.scopebuffer

 Stepping up as a review manager and turning this into initial review thread.
 Base discussion period is until Feb 17. After that I will make summary of
 concerns raised for you to either address or reject with own comment and make
 decision if it is ready to go through voting.


Thanks for taking care of this.
Feb 08 2014
prev sibling next sibling parent reply "Jakob Ovrum" <jakobovrum gmail.com> writes:
On Friday, 7 February 2014 at 08:24:39 UTC, Walter Bright wrote:

 https://github.com/D-Programming-Language/phobos/pull/1911

 This adds a package std.buffer, and the first entry in that 
 package, std.buffer.scopebuffer.

 ScopeBuffer is an OutputRange that sits on the stack, and 
 overflows to malloc/free. It's designed to help in eliminating 
 gc allocation by lower level functions that return buffers, 
 such as std.path.buildPath(). With some judicious user tuning 
 of the initial stack size, it can virtually eliminate storage 
 allocation.

 Using it is  system, but the user of ScopeBuffer can be 
  trusted.

 An output range like this is a precursor to eliminating the 
 excessive gc use by functions such as buildPath().


Wasn't there a simple composed Allocator from std.allocator that 
did exactly this? Composing InSituAllocator with Mallocator 
through FallbackAllocator or something.
Feb 07 2014
next sibling parent Walter Bright <newshound2 digitalmars.com> writes:
On 2/7/2014 3:05 AM, Jakob Ovrum wrote:

 Wasn't there a simple composed Allocator from std.allocator that did exactly
 this? Composing InSituAllocator with Mallocator through FallbackAllocator or
 something.


I don't know anything about that.
Feb 07 2014
prev sibling parent "Dicebot" <public dicebot.lv> writes:
On Friday, 7 February 2014 at 11:05:08 UTC, Jakob Ovrum wrote:

 Wasn't there a simple composed Allocator from std.allocator 
 that did exactly this? Composing InSituAllocator with 
 Mallocator through FallbackAllocator or something.


Yes, looks very similar. I'd like to preserve ScopeBuffer output 
range API but express it internally in terms of std.allocator - 
will also be good field test for performance of the latter.
Feb 07 2014
prev sibling next sibling parent Jacob Carlborg <doob me.com> writes:
On 2014-02-07 09:24, Walter Bright wrote:

 https://github.com/D-Programming-Language/phobos/pull/1911

 This adds a package std.buffer, and the first entry in that package,
 std.buffer.scopebuffer.

 ScopeBuffer is an OutputRange that sits on the stack, and overflows to
 malloc/free. It's designed to help in eliminating gc allocation by lower
 level functions that return buffers, such as std.path.buildPath(). With
 some judicious user tuning of the initial stack size, it can virtually
 eliminate storage allocation.

 Using it is  system, but the user of ScopeBuffer can be  trusted.

 An output range like this is a precursor to eliminating the excessive gc
 use by functions such as buildPath().


Since this is a completely new module, even new package, this should go 
through the standard review process. That means:

* Add to review queue
* Generate documentation
* Do not create a pull request until the review is complete and the 
module has been approved. This easy to to a comparison with Github 
without having to create a pull request

-- 
/Jacob Carlborg
Feb 07 2014
prev sibling next sibling parent reply "Dicebot" <public dicebot.lv> writes:
By the way there was proposal to set up separate mail list for 
review announcements - who can do it?
Feb 08 2014
next sibling parent reply "Brad Anderson" <eco gnuk.net> writes:
On Saturday, 8 February 2014 at 19:37:24 UTC, Dicebot wrote:

 By the way there was proposal to set up separate mail list for 
 review announcements - who can do it?


I'd much rather it be on the newsgroup personally.  It's annoying 
having to
check both email and the forums.
Feb 08 2014
parent "Dicebot" <public dicebot.lv> writes:
On Saturday, 8 February 2014 at 19:55:48 UTC, Brad Anderson wrote:

 On Saturday, 8 February 2014 at 19:37:24 UTC, Dicebot wrote:

 By the way there was proposal to set up separate mail list for 
 review announcements - who can do it?


 I'd much rather it be on the newsgroup personally.  It's 
 annoying having to
 check both email and the forums.


NG is OK too (afaik there is mail list gate)
Feb 08 2014
prev sibling parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Saturday, February 08, 2014 19:37:23 Dicebot wrote:

 By the way there was proposal to set up separate mail list for
 review announcements - who can do it?


I expect that that would make them _less_ visible, as only a certain 
percentage of people will look at that, and there will be plenty of people who 
might look at it if they knew about it but won't initially. We already have 
the announce group for announcements. Why create a new announce group just for 
reviews - especially when review announcements aren't all that frequent? Even 
if we were having reviews as fast as we could, it would typically be 2+ weeks 
between posts.

And if you're proposing that the review itself be done in another list, again, 
that will cut down on the number of people who see it or pay attention to it.

I see no reason not to just use the newsgroups as they are and send review 
announcements to digitalmars-d-announce and the reviews be in digitalmars-d. 
What's wrong with continuing to do it that way?

- Jonathan M Davis
Feb 08 2014
parent "Dicebot" <public dicebot.lv> writes:
On Sunday, 9 February 2014 at 00:20:32 UTC, Jonathan M Davis 
wrote:

 On Saturday, February 08, 2014 19:37:23 Dicebot wrote:

 By the way there was proposal to set up separate mail list for
 review announcements - who can do it?


 I expect that that would make them _less_ visible, as only a 
 certain
 percentage of people will look at that, and there will be 
 plenty of people who
 might look at it if they knew about it but won't initially. We 
 already have
 the announce group for announcements. Why create a new announce 
 group just for
 reviews - especially when review announcements aren't all that 
 frequent? Even
 if we were having reviews as fast as we could, it would 
 typically be 2+ weeks
 between posts.


I notice some people mentioning thet have missed start of review 
or voting in almost any such thread. D.announce traffic is not 
that small these days actually, I doubt many will want to 
subscribe to all its posts to just to be notified about reviews. 
I don't propose to move discussion there - just make additional 
notifications identical to those that go to D.announce

In theory it shouldn't be needed but reality has been different 
so far. And I'd really love to get more participation from Phobos 
contributors who don't seem to  follow NG _that_ closely.

Counter proposals are highly welcome ;)
Feb 08 2014
prev sibling next sibling parent reply Jacob Carlborg <doob me.com> writes:
On 2014-02-07 09:24, Walter Bright wrote:

 https://github.com/D-Programming-Language/phobos/pull/1911

 This adds a package std.buffer, and the first entry in that package,
 std.buffer.scopebuffer.


Thinking out loud here:

* Could we put this in a more general package, like 
std.memory.buffer.scopebuffer. Then the upcoming std.allocator could be 
placed in std.memory.allocator

* Do we have any uses for this in druntime? Perhaps move core.memory to 
core.memory.gc and put this module in core.memory.buffer.scopebuffer.

-- 
/Jacob Carlborg
Feb 09 2014
next sibling parent "Dicebot" <public dicebot.lv> writes:
On Sunday, 9 February 2014 at 12:30:56 UTC, Jacob Carlborg wrote:

 * Could we put this in a more general package, like 
 std.memory.buffer.scopebuffer. Then the upcoming std.allocator 
 could be placed in std.memory.allocator


I don't like "buffer" as a package name at all. Probably better 
approach is to put it in same package with Appender and make it 
common package for all standard output range implementations.

Its relation to std.allocator is just a (desired) implementation 
detail.
Feb 09 2014
prev sibling parent Walter Bright <newshound2 digitalmars.com> writes:
On 2/9/2014 4:30 AM, Jacob Carlborg wrote:

 * Could we put this in a more general package, like
 std.memory.buffer.scopebuffer. Then the upcoming std.allocator could be placed
 in std.memory.allocator


In druntime there are many examples of deeply nested packages. Personally, I 
never can remember which one a particular module is in and are always using 
directory searches to find them. I much prefer a flatter structure.

Besides, I don't see that buffers are a subset of memory allocation strategies, 
I think they're orthogonal.



 * Do we have any uses for this in druntime? Perhaps move core.memory to
 core.memory.gc and put this module in core.memory.buffer.scopebuffer.


Of course, the distinction between druntime and phobos is a bit arbitrary, but 
given that lots of buffer types are coming, I don't see much value in having 
some buffer types in druntime and some in phobos.
Feb 09 2014
prev sibling next sibling parent reply Dmitry Olshansky <dmitry.olsh gmail.com> writes:
07-Feb-2014 12:24, Walter Bright пишет:

 https://github.com/D-Programming-Language/phobos/pull/1911

 This adds a package std.buffer, and the first entry in that package,
 std.buffer.scopebuffer.

 ScopeBuffer is an OutputRange that sits on the stack, and overflows to
 malloc/free. It's designed to help in eliminating gc allocation by lower
 level functions that return buffers, such as std.path.buildPath(). With
 some judicious user tuning of the initial stack size, it can virtually
 eliminate storage allocation.

 Using it is  system, but the user of ScopeBuffer can be  trusted.

 An output range like this is a precursor to eliminating the excessive gc
 use by functions such as buildPath().


Some food for thought:

1. I guess there is a distinctive quality to a buffer (i.e. it's not 
just any output range). The current interface looks quite arbitrary - is 
'pop' required for any buffer? A trait like isBuffer is in order, to 
allow user code to target any buffers including non-Phobos ones.

2. May need buffer.package.d to import all common buffers (there is only 
one now, but with time...).


-- 
Dmitry Olshansky
Feb 09 2014
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 2/9/2014 12:43 PM, Dmitry Olshansky wrote:


 1. I guess there is a distinctive quality to a buffer (i.e. it's not just any
 output range). The current interface looks quite arbitrary - is 'pop' required
 for any buffer?


No, pop isn't required.


 A trait like isBuffer is in order, to allow user code to target
 any buffers including non-Phobos ones.


I don't understand the point of this suggestion.


 2. May need buffer.package.d to import all common buffers (there is only one
 now, but with time...).


I do not like the idea of "kitchen sink" importing. The package.d thing was 
designed so that we can split up the existing kitchen sink modules without 
breaking user code.
Feb 09 2014
next sibling parent Dmitry Olshansky <dmitry.olsh gmail.com> writes:
10-Feb-2014 04:59, Walter Bright пишет:

 On 2/9/2014 12:43 PM, Dmitry Olshansky wrote:


 1. I guess there is a distinctive quality to a buffer (i.e. it's not
 just any
 output range). The current interface looks quite arbitrary - is 'pop'
 required
 for any buffer?


 No, pop isn't required.


Then some canonical picture of what *any* Buffer is would be nice. I'm 
having a bit of deja vu with this discussion :)


 A trait like isBuffer is in order, to allow user code to target
 any buffers including non-Phobos ones.


 I don't understand the point of this suggestion.


If buffer is just an output range and 'put' is enough to model it, then 
fine. But see also recent thread on output ranges:
http://forum.dlang.org/thread/atfdzzxutxnduavvzvww forum.dlang.org#post-atfdzzxutxnduavvzvww:40forum.dlang.org

It could be useful to define a more broad interface as a superset of 
output ranges.


 2. May need buffer.package.d to import all common buffers (there is
 only one
 now, but with time...).


 I do not like the idea of "kitchen sink" importing. The package.d thing
 was designed so that we can split up the existing kitchen sink modules
 without breaking user code.


Okay.

-- 
Dmitry Olshansky
Feb 10 2014
prev sibling parent reply "Dicebot" <public dicebot.lv> writes:
On Monday, 10 February 2014 at 00:59:00 UTC, Walter Bright wrote:

 2. May need buffer.package.d to import all common buffers 
 (there is only one
 now, but with time...).


 I do not like the idea of "kitchen sink" importing. The 
 package.d thing was designed so that we can split up the 
 existing kitchen sink modules without breaking user code.


It may be discouraged but in the end it is up to user to decide. 
I think all packages in Phobos must mandatory provide package.d

In general, proposal to standard library is a place where 
defining common rules (and conforming to them) has higher 
priority over any common sense.
Feb 10 2014
next sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 2/10/2014 4:24 AM, Dicebot wrote:

 In general, proposal to standard library is a place where defining common rules
 (and conforming to them) has higher priority over any common sense.


I cannot agree with following rules to the point of throwing common sense out 
the window.
Feb 10 2014
parent reply "Francesco Cattoglio" <francesco.cattoglio gmail.com> writes:
On Monday, 10 February 2014 at 19:31:50 UTC, Walter Bright wrote:

 I cannot agree with following rules to the point of throwing 
 common sense out the window.


If following the rules requires throwing common sense out of the 
window, perhaps it's time to change the rules.
Feb 10 2014
parent reply "Dicebot" <public dicebot.lv> writes:
On Monday, 10 February 2014 at 19:36:40 UTC, Francesco Cattoglio 
wrote:

 On Monday, 10 February 2014 at 19:31:50 UTC, Walter Bright 
 wrote:

 I cannot agree with following rules to the point of throwing 
 common sense out the window.


 If following the rules requires throwing common sense out of 
 the window, perhaps it's time to change the rules.


Exactly. And it should be done in a separate proposal and with 
own discussion. Which can possibly reveal that common senses 
differ a lot and thus is necessary.

I personally will vote "No" for any new proposal that looks 
obviously alien from existing Phobos code, despite it being 
possibly very useful and desired and backed by sound reasoning.
Feb 10 2014
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 2/10/2014 11:46 AM, Dicebot wrote:

 I personally will vote "No" for any new proposal that looks obviously alien
from
 existing Phobos code, despite it being possibly very useful and desired and
 backed by sound reasoning.


1. There are many, many package folders in phobos/druntime that do not have a 
package.d.

2. package.d can always be added. But it cannot be subtracted. Hence, it is
best 
not to add it "just because", it needs more compelling arguments.
Feb 10 2014
parent reply "Dicebot" <public dicebot.lv> writes:
On Tuesday, 11 February 2014 at 00:31:32 UTC, Walter Bright wrote:

 On 2/10/2014 11:46 AM, Dicebot wrote:

 I personally will vote "No" for any new proposal that looks 
 obviously alien from
 existing Phobos code, despite it being possibly very useful 
 and desired and
 backed by sound reasoning.


 1. There are many, many package folders in phobos/druntime that 
 do not have a package.d.

 2. package.d can always be added. But it cannot be subtracted. 
 Hence, it is best not to add it "just because", it needs more 
 compelling arguments.


Oh, this was not related to package.d (it is a new thing and 
can't have established guidelines) - more of a general statement. 
For example, you have been arguing in PR to keep `lwr` and `upr` 
identifiers while quick grep shows that there is not a single 
place in Phobos which uses such naming scheme in public API. Such 
things are not related to any architectural choices - those are 
personal preferences and thus is less important than existing 
style guidelines by definition.
Feb 11 2014
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 2/11/2014 5:30 AM, Dicebot wrote:

 For example, you have
 been arguing in PR to keep `lwr` and `upr` identifiers while quick grep shows
 that there is not a single place in Phobos which uses such naming scheme in
 public API.



grep -i -r lwr *.d
std\traits.d:        alias G Lwr;
std\traits.d:            enum ok = functionLinkage!Upr == functionLinkage!Lwr;
std\traits.d:            enum ok = variadicFunctionStyle!Upr == 
variadicFunctionStyle!Lwr;
std\traits.d:            // Note the order of arguments.  The convertion order 
Lwr -> Upr is
std\traits.d:            // correct since Upr should be semantically 'narrower' 
than Lwr.
std\traits.d:            enum ok = isStorageClassImplicitlyConvertible!(Lwr,
Upr);
std\traits.d:            enum lwrAtts = functionAttributes!Lwr;
std\traits.d:                (  (uprAtts & wantExact)   == (lwrAtts &
wantExact)) &&
std\traits.d:                (  (uprAtts & FA.pure_   ) >= (lwrAtts & FA.pure_ 
  )) &&
std\traits.d:                (  (uprAtts & FA.nothrow_) >= (lwrAtts & 
FA.nothrow_)) &&
std\traits.d:                (!!(uprAtts & safety    )  >= !!(lwrAtts & safety 
   )) ;
std\traits.d:            enum ok = is(ReturnType!Upr : ReturnType!Lwr);
std\traits.d:            alias ParameterTypeTuple!Lwr LwrParams;
std\traits.d:            alias ParameterStorageClassTuple!Lwr LwrPSTCs;
std\traits.d:                    enum lwrStc = LwrPSTCs[i];
std\traits.d:                        ((uprStc & wantExact )  == (lwrStc & 
wantExact )) &&
std\traits.d:                        ((uprStc & STC.scope_)  >= (lwrStc & 
STC.scope_)) &&
std\traits.d:            static if (UprParams.length == LwrParams.length)
std\traits.d:                enum ok = is(UprParams == LwrParams) && 
checkNext!(0).ok;
std\typetuple.d: * type tuple. TL[$(I lwr) .. $(I upr)] returns a new type
Feb 12 2014
parent reply "Dicebot" <public dicebot.lv> writes:
On Wednesday, 12 February 2014 at 18:50:24 UTC, Walter Bright 
wrote:

 grep -i -r lwr *.d
 std\traits.d:        alias G Lwr;
 std\traits.d:            enum ok = functionLinkage!Upr == 
 functionLinkage!Lwr;
 std\traits.d:            enum ok = variadicFunctionStyle!Upr == 
 variadicFunctionStyle!Lwr;
 std\traits.d:            // Note the order of arguments.  The 
 convertion order Lwr -> Upr is
 std\traits.d:            // correct since Upr should be 
 semantically 'narrower' than Lwr.
 std\traits.d:            enum ok = 
 isStorageClassImplicitlyConvertible!(Lwr, Upr);
 std\traits.d:            enum lwrAtts = functionAttributes!Lwr;
 std\traits.d:                (  (uprAtts & wantExact)   == 
 (lwrAtts & wantExact)) &&
 std\traits.d:                (  (uprAtts & FA.pure_   ) >= 
 (lwrAtts & FA.pure_
  )) &&
 std\traits.d:                (  (uprAtts & FA.nothrow_) >= 
 (lwrAtts & FA.nothrow_)) &&
 std\traits.d:                (!!(uprAtts & safety    )  >= 
 !!(lwrAtts & safety
   )) ;
 std\traits.d:            enum ok = is(ReturnType!Upr : 
 ReturnType!Lwr);
 std\traits.d:            alias ParameterTypeTuple!Lwr LwrParams;
 std\traits.d:            alias ParameterStorageClassTuple!Lwr 
 LwrPSTCs;
 std\traits.d:                    enum lwrStc = LwrPSTCs[i];
 std\traits.d:                        ((uprStc & wantExact )  == 
 (lwrStc & wantExact )) &&
 std\traits.d:                        ((uprStc & STC.scope_)  >= 
 (lwrStc & STC.scope_)) &&
 std\traits.d:            static if (UprParams.length == 
 LwrParams.length)
 std\traits.d:                enum ok = is(UprParams == 
 LwrParams) && checkNext!(0).ok;
 std\typetuple.d: * type tuple. TL[$(I lwr) .. $(I upr)] returns 
 a new type


Exactly - all those places are internals of few very old 
templates ;) Other than one part of documentation in typetuple, 
have not noticed it initially. It is not even comparable with 
grep results for "lower" and "upper". I'd expect those places in 
std.traits to be fixed too once someone will be working on that 
part of code.
Feb 12 2014
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 2/12/2014 10:55 AM, Dicebot wrote:

 I'd expect those
 places in std.traits to be fixed too once someone will be working on that part
 of code.


Worrying about "lwr" vs "lower" is so way, way far down the list of what we 
should be paying attention to it is not even visible.
Feb 12 2014
next sibling parent "Jakob Ovrum" <jakobovrum gmail.com> writes:
On Wednesday, 12 February 2014 at 21:01:26 UTC, Walter Bright 
wrote:

 On 2/12/2014 10:55 AM, Dicebot wrote:

 I'd expect those
 places in std.traits to be fixed too once someone will be 
 working on that part
 of code.


 Worrying about "lwr" vs "lower" is so way, way far down the 
 list of what we should be paying attention to it is not even 
 visible.


Wording and naming in public interfaces is very important, which 
I'm sure you agree with.

it wouldn't have stolen anyone's attention if you'd just changed 
it as soon as it was mentioned. It was already demonstrated that 
the full `lower`/`upper` is far more common in Phobos than the 
hardly used abbreviations, so what's the big deal?
Feb 12 2014
prev sibling next sibling parent reply "Dicebot" <public dicebot.lv> writes:
On Wednesday, 12 February 2014 at 21:01:26 UTC, Walter Bright 
wrote:

 On 2/12/2014 10:55 AM, Dicebot wrote:

 I'd expect those
 places in std.traits to be fixed too once someone will be 
 working on that part
 of code.


 Worrying about "lwr" vs "lower" is so way, way far down the 
 list of what we should be paying attention to it is not even 
 visible.


How can one review something more important if you ignore even 
basic style comments?
Feb 12 2014
parent Walter Bright <newshound2 digitalmars.com> writes:
On 2/12/2014 1:18 PM, Dicebot wrote:

 How can one review something more important if you ignore even basic style
 comments?


Disagreeing with it is not ignoring it.
Feb 12 2014
prev sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 2/12/14, 1:01 PM, Walter Bright wrote:

 On 2/12/2014 10:55 AM, Dicebot wrote:

 I'd expect those
 places in std.traits to be fixed too once someone will be working on
 that part
 of code.


 Worrying about "lwr" vs "lower" is so way, way far down the list of what
 we should be paying attention to it is not even visible.


If they're in the public interface of your pull request and a reviewer 
asked you to make the change, make the change. It's a sensible request 
and making the change is the one civilized reply. Not doing so and 
insisting it's irrelevant is wasting everybody's time to a much larger 
extent.

Andrei
Feb 12 2014
prev sibling parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Monday, February 10, 2014 12:24:24 Dicebot wrote:

 On Monday, 10 February 2014 at 00:59:00 UTC, Walter Bright wrote:

 2. May need buffer.package.d to import all common buffers
 (there is only one
 now, but with time...).


 
 I do not like the idea of "kitchen sink" importing. The
 package.d thing was designed so that we can split up the
 existing kitchen sink modules without breaking user code.


 
 It may be discouraged but in the end it is up to user to decide.
 I think all packages in Phobos must mandatory provide package.d


That would be _encouraging_ the use of importing whole packages at once, which 
I don't think that we're looking to do. If some folks want to do that with 
their own projects, fine, but the feature was introduced specifically in order 
to split of larger modules into packages, not to treat packages as modules. 
And on top of that, as has come a number of times previously when this sort of 
thing has been discussed, Phobos was not designed with the idea that you would 
import entire packages at once, and it could cause some entertaining symbol 
conflicts, which people would then complain about if we tried to support 
importing Phobos packages in general. Also, even if there weren't any 
conflicts now, we'd have to worry about additional conflicts every time that 
we added a symbol to Phobos. At the extreme, if std had a package.d file that 
imported _everything_ so that you could just import std; and have all of 
Phobos, then adding any symbol to any module in all of Phobos where there was 
another symbol with that same name in another, unrelated module would then 
break existing code.

I'm with Walter on this one. We shouldn't be adding any package.d files to 
Phobos except when splitting up existing modules. And even then, down the 
line, we might want to consider deprecating the imports in the package.d files 
that we introduce when splitting up modules so that existing code moves 
towards importing the new modules directly. But at minimum, we don't want to 
be introducing package.d files for existing packages. That's just asking for 
trouble.

- Jonathan M Davis
Feb 10 2014
next sibling parent reply "Dicebot" <public dicebot.lv> writes:
On Monday, 10 February 2014 at 22:42:30 UTC, Jonathan M Davis 
wrote:

 That would be _encouraging_ the use of importing whole packages 
 at once, which
 I don't think that we're looking to do. If some folks want to 
 do that with
 their own projects, fine, but the feature was introduced 
 specifically in order
 to split of larger modules into packages, not to treat packages 
 as modules.


Prohibiting alternative is not "discouraging", it is prohibiting. 
Discouraging is providing both options and clearly stating which 
one is recommended. So far D has been a very permissive language 
in terms of user choice, it is strange to suddenly start making 
restrictions here.
Feb 10 2014
parent Walter Bright <newshound2 digitalmars.com> writes:
On 2/10/2014 2:45 PM, Dicebot wrote:

 Prohibiting alternative is not "discouraging", it is prohibiting. Discouraging
 is providing both options and clearly stating which one is recommended. So far
D
 has been a very permissive language in terms of user choice, it is strange to
 suddenly start making restrictions here.


Often more than one way to do something in Phobos is tolerated for backwards 
compatibility reasons, not because it is a good idea on its own.

One old case comes to mind, where the author of a module wanted to provide both 
an OOP interface and a free function interface to the same functionality, in
the 
name of "user choice". To me, this is giving some false illusion of
flexibility, 
when in fact it is merely clutter and bloat.
Feb 10 2014
prev sibling parent "Meta" <jared771 gmail.com> writes:
On Monday, 10 February 2014 at 22:42:30 UTC, Jonathan M Davis 
wrote:

 And on top of that, as has come a number of times previously 
 when this sort of
 thing has been discussed, Phobos was not designed with the idea 
 that you would
 import entire packages at once, and it could cause some 
 entertaining symbol
 conflicts, which people would then complain about if we tried 
 to support
 importing Phobos packages in general. Also, even if there 
 weren't any
 conflicts now, we'd have to worry about additional conflicts 
 every time that
 we added a symbol to Phobos. At the extreme, if std had a 
 package.d file that
 imported _everything_ so that you could just import std; and 
 have all of
 Phobos, then adding any symbol to any module in all of Phobos 
 where there was
 another symbol with that same name in another, unrelated module 
 would then
 break existing code.


I absolutely hate getting symbol conflicts when importing 
multiple Phobos modules, but I don't think importing an entire 
package as a module is all that unreasonable. That's basically 
what we're doing right now, as Phobos is so flat. Your example of 
importing all of std is a bit extreme, but if we can't even 
support reasonable cases, then either D's module system, its 
design, or both have failed in this regard. One of the worst 
things about the module system currently is the private symbol 
conflicts, and fixing that would go a long way to allowing some 
real modularity in Phobos, as well as splitting up the different 
packages and factoring out any common functionality into 
privately-imported modules.
Feb 10 2014
prev sibling parent "Dicebot" <public dicebot.lv> writes:
I have created a small summary for this proposal based on 
existing comments : http://wiki.dlang.org/Std.buffer.scopebuffer

By this point I consider initial discussion finished and from now 
it is up to Walter to prepare his proposal to formal review for 
Phobos inclusion.

Walter, please write me an e-mail (public-at-dicebot-lv) when you 
find yourself ready to next step. There are some concerns left 
that you can find down the abovementioned link - for most of 
those it is up to your personal judgement if anything needs to be 
changed about it.

However there is a single mandatory condition that needs to be 
met before I can start formal review thread - documentation. All 
Phobos proposals must have pre-generated documentation hosted 
somewhere and available for quick inspection of reviewers. DDOC 
also should cover all module details - not only plain function 
descriptions but also function parameters and return types.
Feb 17 2014