On 12 April 2013 14:46, Jesse Phillips <Jessekphillips+d@gmail.com> wrote:

> It is that time, If you would like to see the proposed std.process include into Phobos please vote yes. If one condition must be met specify under what condition, otherwise vote no.
>

I didn't think I had much of an opinion on std.process, but I just gave it a quick once over and noticed, relating to our recent discussions about flagrant GC usage throughout phobos, that this is riddled with the exact sorts of issues I've been talking about:

string[string] is used in the main API to receive environment variables; perhaps kinda convenient, but it's impossible to supply environment variables with loads of allocations.

toStringz is used liberally; alternatively, alloca() could allocate the c-string's on the stack and zero terminate them there, passing a pointer to the stack string to the OS functions.

String concatenation is rampant! Look at this code to parse the env variables (which are already an AA):

    foreach (var, val; childEnv)
        envz[pos++] = (var~'='~val~'\0').ptr;


And many many more of the same...

This is a lib that spawns a process, I can't imagine why this library
should need to allocate a single byte, but it allocates wildly just to
perform a trivial pass-through to the OS.
This module is no exception either, basically all of phobos is like this,
and I'd like to see careful consideration made to these details in the
future.

I've said before, I sadly have to avoid phobos like the plague. Some modules (like this one) that provide fundamental functionality - not just helper functions - can't be avoided. Requirements for those should be extra strict in my opinion.

On Friday, 12 April 2013 at 07:04:23 UTC, Manu wrote:
> And many many more of the same...

So how many of these will produce a measurable performance increase when optimized, considering the relative cost of process creation?

> This is a lib that spawns a process, I can't imagine why this library should need to allocate a single byte

So that the library fits in 3500 lines instead of many more, and has an API that can be invoked using one line, instead of ten.

On Friday, 12 April 2013 at 07:00:25 UTC, Vladimir Panteleev wrote:
> On Friday, 12 April 2013 at 06:25:10 UTC, Manu wrote:
>> Can I suggest that ALL new modules should be added to exp. rather than
>> std.?
>
> Java tried this. As a result, they are now stuck with "javax" because they realized breaking backwards compatibility wasn't worth the benefit of "graduating" packages from "javax" to "java".

Actually they also have another issue.

Java supported since the begging something similar to a System
package for dirty tricks, officially it was a Sun only API.

http://www.docjar.com/docs/api/sun/misc/Unsafe.html

So actually all Java implementations provide a compatible API, because many do use it.

Younger languages seem not to have any issue breaking backwards compatibility for experimental code.

--
Paulo

On 12 April 2013 17:13, Vladimir Panteleev <vladimir@thecybershadow.net>wrote:

> On Friday, 12 April 2013 at 07:04:23 UTC, Manu wrote:
>
>> And many many more of the same...
>>
>
> So how many of these will produce a measurable performance increase when optimized, considering the relative cost of process creation?


I agree that spawning processes is a low-frequency operation, but it's a principle I'm trying to illustrate here.


 This is a lib that spawns a process, I can't imagine why this library
>> should need to allocate a single byte
>>
>
> So that the library fits in 3500 lines instead of many more, and has an API that can be invoked using one line, instead of ten.
>

You can argue whatever you like. I've said my part, and I wherever it lands
is not my call.
I'm just illustrating the point that phobos, like STL, and to a lesser
extent the CRT, are to continue to be avoided like the plague in some
fields unless these details are considered in future.

Most of the required changes would be self-contained, and not affect the API at all.

On Friday, 12 April 2013 at 07:22:42 UTC, Manu wrote:
> I agree that spawning processes is a low-frequency operation, but it's a
> principle I'm trying to illustrate here.

My point was that it is not that it's low-frequency, it's that the OS process creation operation is so expensive, that a few memory allocations will not make much of a difference in comparison. It's the same as optimizing memory allocations in a program which is intrinsically disk- or network-bound.

> You can argue whatever you like. I've said my part, and I wherever it lands is not my call.

Well, that's just not very constructive.

Your complaint in valid in general, but I was pointing out that it is not much so when specifically aimed at std.process.

On Fri, 12 Apr 2013 03:04:08 -0400, Manu <turkeyman@gmail.com> wrote:

> On 12 April 2013 14:46, Jesse Phillips <Jessekphillips+d@gmail.com> wrote:
>
>> It is that time, If you would like to see the proposed std.process include
>> into Phobos please vote yes. If one condition must be met specify under
>> what condition, otherwise vote no.
>>
>
> I didn't think I had much of an opinion on std.process, but I just gave it
> a quick once over and noticed, relating to our recent discussions about
> flagrant GC usage throughout phobos, that this is riddled with the exact
> sorts of issues I've been talking about:

It's spawning a new process.  What is the issue with allocating a bit of memory?  The spawning of the process performance is going to dwarf that of any memory allocations.

> string[string] is used in the main API to receive environment variables;
> perhaps kinda convenient, but it's impossible to supply environment
> variables with loads of allocations.

I think you meant "without"?

What would be your suggestion?  string[string] is the built-in map type.  How do you pass an environment map without having some allocations?

> toStringz is used liberally; alternatively, alloca() could allocate the
> c-string's on the stack and zero terminate them there, passing a pointer to
> the stack string to the OS functions.

This would be a lot of effort for pretty much zero gain for the majority of cases.

> String concatenation is rampant! Look at this code to parse the env
> variables (which are already an AA):
>
>     foreach (var, val; childEnv)
>         envz[pos++] = (var~'='~val~'\0').ptr;

This could be improved.  It could also be optimized into a single allocation automatically by the compiler (it might already be).  The API would not be affected by this improvement, though.

> And many many more of the same...
>
> This is a lib that spawns a process, I can't imagine why this library
> should need to allocate a single byte, but it allocates wildly just to
> perform a trivial pass-through to the OS.

It is not a trivial pass-through.  Different OSes require different parameter types.  Look at the difference between the windows and posix implementations.  We are writing a cross-platform library, so whatever we pick will have to be converted on at least one OS.

> This module is no exception either, basically all of phobos is like this,
> and I'd like to see careful consideration made to these details in the
> future.

I too like to avoid memory allocations.  But the level to which you require is too stringent.  Not even Tango, which made low heap-allocations a priority, avoided allocations in it's process library.

> I've said before, I sadly have to avoid phobos like the plague. Some
> modules (like this one) that provide fundamental functionality - not just
> helper functions - can't be avoided. Requirements for those should be extra
> strict in my opinion.

We cannot cater to all developers.  To put all developers through a lot of pain in order to attract the fringe ones is not a practical goal.

I think the convenience of std.process is what it is there for.  For the highest performance, you are able to call the OS functions directly.

-Steve

On 12 April 2013 17:30, Vladimir Panteleev <vladimir@thecybershadow.net>wrote:

> On Friday, 12 April 2013 at 07:22:42 UTC, Manu wrote:
>
>> I agree that spawning processes is a low-frequency operation, but it's a principle I'm trying to illustrate here.
>>
>
> My point was that it is not that it's low-frequency, it's that the OS process creation operation is so expensive, that a few memory allocations will not make much of a difference in comparison. It's the same as optimizing memory allocations in a program which is intrinsically disk- or network-bound.


Which OS are we talking about?
What OS runs on an a Nintendo Wii? There's only 24mb of system memory in
that machine, can we afford to allocate it frivolously?

Will I avoid phobos as a policy? Yes.


 You can argue whatever you like. I've said my part, and I wherever it
>> lands is not my call.
>>
>
> Well, that's just not very constructive.
>
> Your complaint in valid in general, but I was pointing out that it is not much so when specifically aimed at std.process.
>

I don't necessarily disagree with your point (although I don't agree
either). Perhaps in the context of std.process it's not so important... but
it's still an opportunity to set a precedent.
Honestly, any new module could have appeared for approval at this
particular moment and I would have made the same criticisms. Not
necessarily picking on std.process in particular, I'm making a point about
phobos, and what is considered acceptable.

Yes.

On 12 April 2013 17:35, Steven Schveighoffer <schveiguy@yahoo.com> wrote:

> On Fri, 12 Apr 2013 03:04:08 -0400, Manu <turkeyman@gmail.com> wrote:
>
 string[string] is used in the main API to receive environment variables;
>> perhaps kinda convenient, but it's impossible to supply environment variables with loads of allocations.
>>
>
> I think you meant "without"?
>

Yes.


What would be your suggestion?  string[string] is the built-in map type.
>  How do you pass an environment map without having some allocations?


I'd use string[].


 toStringz is used liberally; alternatively, alloca() could allocate the
>> c-string's on the stack and zero terminate them there, passing a pointer
>> to
>> the stack string to the OS functions.
>>
>
> This would be a lot of effort for pretty much zero gain for the majority of cases.


A trivial ... mixin template (?) could wrap it up, I don't see it
particularly less convenient than calling toStringz().
Perhaps there are other tools missing from phobos if this is somehow hard...


 String concatenation is rampant! Look at this code to parse the env
>> variables (which are already an AA):
>>
>>     foreach (var, val; childEnv)
>>         envz[pos++] = (var~'='~val~'\0').ptr;
>>
>
> This could be improved.  It could also be optimized into a single allocation automatically by the compiler (it might already be).  The API would not be affected by this improvement, though.
>

I've never seem the compiler apply that optimisation, although I often wish
it would.
I saw an appender appear a few pages below, that would be an improvement
here too I guess.


 And many many more of the same...
>>
>> This is a lib that spawns a process, I can't imagine why this library should need to allocate a single byte, but it allocates wildly just to perform a trivial pass-through to the OS.
>>
>
> It is not a trivial pass-through.  Different OSes require different parameter types.  Look at the difference between the windows and posix implementations.  We are writing a cross-platform library, so whatever we pick will have to be converted on at least one OS.


Writing cross-platform code is my life.
They are trivial conversions. It could be done on the stack easily.
Granted, ideally the compiler could theoretically perform some of those
improvements, but it doesn't, and probably depends on other changes to be
able to do so anyway.
For instance, the compiler couldn't confidently lower the allocations to
the stack unless 'scope' arguments worked, so it knew it wouldn't escape
the OS call it was being handed to.


 This module is no exception either, basically all of phobos is like this,
>> and I'd like to see careful consideration made to these details in the future.
>>
>
> I too like to avoid memory allocations.  But the level to which you require is too stringent.  Not even Tango, which made low heap-allocations a priority, avoided allocations in it's process library.


But it would be trivial to use the stack in these cases. It doesn't matter if it adds a few lines under the hood.

I'm really just trying to make the point that it should ALWAYS be a key
consideration, and people should make it habit to think about/criticise
these things when accepting code into phobos.
This is the standard library, it will be used in more D code than any other
library ever. Why skimp here?


 I've said before, I sadly have to avoid phobos like the plague. Some
>> modules (like this one) that provide fundamental functionality - not just
>> helper functions - can't be avoided. Requirements for those should be
>> extra
>> strict in my opinion.
>>
>
> We cannot cater to all developers.


In this case you certainly can, it would be fairly trivial to do.


 To put all developers through a lot of pain in order to attract the fringe
> ones is not a practical goal.
>

What pain? Are you saying a stack variable is somehow hard?
If that's the case, then there's clearly some other fundamental tools
missing from phobos.


I think the convenience of std.process is what it is there for.  For the
> highest performance, you are able to call the OS functions directly.
>

No, it's to save (ideally) _all_ programmers the effort of rewriting this
module themselves.
This is exactly the sort of thing I waste loads of my life doing, rewriting
wheels because the author was a PC programmer, and simply didn't give a
shit.
What a waste of life >_<

I re-iterate, maybe my points aren't of critical importance in relation to std.process specifically, I'm trying to raise some general issues with phobos submissions (that shouldn't exclude std.process either).

On Friday, 12 April 2013 at 07:53:04 UTC, Manu wrote:
> Which OS are we talking about?
> What OS runs on an a Nintendo Wii? There's only 24mb of system memory in
> that machine, can we afford to allocate it frivolously?

I wasn't aware that writing games for the Wii involves creating processes in the thousands.

> I don't necessarily disagree with your point (although I don't agree
> either). Perhaps in the context of std.process it's not so important... but
> it's still an opportunity to set a precedent.
> Honestly, any new module could have appeared for approval at this
> particular moment and I would have made the same criticisms. Not
> necessarily picking on std.process in particular, I'm making a point about
> phobos, and what is considered acceptable.

Don't forget that shifting focus when adjusting goals is always a question of tradeoffs. Sacrificing tersity, maintainability, usability, safety etc. for the sake of performance would be a poor move in the case of std.process.