www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Truly nogc Exceptions?

reply Steven Schveighoffer <schveiguy gmail.com> writes:
Given dip1008, we now can throw exceptions inside  nogc code! This is 
really cool, and helps make code that uses exceptions or errors  nogc. 
Except...

The mechanism to report what actually went wrong for an exception is a 
string passed to the exception during *construction*. Given that you 
likely want to make such an exception inside a  nogc function, you are 
limited to passing a compile-time-generated string (either a literal or 
one generated via CTFE).

To demonstrate what I mean, let me give you an example member function 
inside a type containing 2 fields, x and y:

void foo(int[] arr)
{
    auto x = arr[x .. y];
}

There are 2 ways this can throw a range error:

a) x > y
b) y > arr.length

But which is it? And what are x and y, or even the array length?

The error message we get is basic (module name and line number aren't 
important here):

    core.exception.RangeError testerror.d(6): Range violation

Not good enough -- we have all the information present to give a more 
detailed message. Why not:

    Attempted slice with wrong ordered parameters, 5 .. 4

or

    Slice parameter 6 is greater than length 5

All that information is available, yet we don't see anything like that.

Let's look at the base of all exception and error types to see why we 
don't have such a thing. The part which prints this message is the 
member function toString inside Throwable, repeated here for your 
reading pleasure [1]:

     void toString(scope void delegate(in char[]) sink) const
     {
         import core.internal.string : unsignedToTempString;

         char[20] tmpBuff = void;

         sink(typeid(this).name);
         sink(" "); sink(file);
         sink("("); sink(unsignedToTempString(line, tmpBuff, 10)); 
sink(")");

         if (msg.length)
         {
             sink(": "); sink(msg);
         }
         if (info)
         {
             try
             {
                 sink("\n----------------");
                 foreach (t; info)
                 {
                     sink("\n"); sink(t);
                 }
             }
             catch (Throwable)
             {
                 // ignore more errors
             }
         }
     }

(Side Note: there is an overload for toString which takes no delegate 
and returns a string. But since this overload is present, doing e.g. 
writeln(myEx) will use it)

Note how this *doesn't* allocate anything.

But hang on, what about the part that actually prints the message:

         sink(typeid(this).name);
         sink(" "); sink(file);
         sink("("); sink(unsignedToTempString(line, tmpBuff, 10)); 
sink(")");

         if (msg.length)
         {
             sink(": "); sink(msg);
         }

Hm... Note how the file name, and the line number are all *members* of 
the exception, and there was no need to allocate a special string to 
contain the message we saw. So it *is* possible to have a custom message 
without allocation. It's just that the only interface for details is via 
the `msg` string member field -- which is only set on construction.

We can do better.

I noticed that there is a  __future member function inside Throwable 
called message. This function returns the message that the Throwable is 
supposed to display (defaulting to return msg). I believe this was 
inserted at Sociomantic's request, because they need to be able to have 
a custom message rendered at *print* time, not *construction* time [2]. 
This makes sense -- why do we need to allocate some string that will 
never be printed (in the case where an exception is caught and handled)? 
This helps alleviate the problem a bit, as we could construct our 
message at print-time when the  nogc requirement is no longer present.

But we can do even better.

What if we added ALSO a function:

void message(scope void delegate(in char[]) sink)

In essence, this does *exactly* what the const(char)[] returning form of 
message does, but it doesn't require any allocation, nor storage of the 
data to print inside the exception. We can print numbers (and other 
things) and combine them together with strings just like the toString 
function does.

We can then replace the code for printing the message inside toString 
with this:

        bool printedColon = false;
        void subSink(in char[] data)
        {
           if(!printedColon && data.length > 0)
           {
               sink(": ");
               printedColon = true;
           }
           sink(data);
        }
        message(&subSink);

In this case, we then have a MUCH better mechanism to implement our 
desired output from the slice error:

class RangeSliceError : Throwable
{
     size_t lower;
     size_t upper;
     size_t len;

     ...

     override void message(scope void delegate(in char[]) sink)
     {
         import core.internal.string : unsignedToTempString;

         char[20] tmpBuff = void;

         if (lower > upper)
         {
            sink("Attempted slice with wrong ordered parameters ");
            sink(unsignedToTempString(lower, tmpBuff, 10));
            sink(" .. ");
            sink(unsignedToTempString(upper, tmpBuff, 10));
         }
         else if (upper > len)
         {
            sink("Slice parameter ");
            sink(unsignedToTempString(upper, tmpBuff, 10));
            sink(" is greater than length ");
            sink(unsignedToTempString(len, tmpBuff, 10));
         }
         else // invalid parameters to this class
            sink("Slicing Error, but unsure why");
     }
}

And look Ma, no allocations!

So we can truly have  nogc exceptions, without having requirements to 
use the GC on construction. I think we should add this.

One further thing: I didn't make the sink version of message  nogc, but 
in actuality, it could be. Notice how it allocates using the stack. Even 
if we needed some indeterminate amount of memory, it would be simple to 
use C malloc/free, or alloca. But traditionally, we don't put any 
attributes on these base functions. Would it make sense in this case?

As Andrei says -- Destroy!

-Steve

[1] - 
https://github.com/dlang/druntime/blob/542b680f2c2e09e7f4b494898437c61216583fa5/src/object.d#L2642
[2] - https://github.com/dlang/druntime/pull/1895
Sep 19 2018
next sibling parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 9/19/18 5:16 PM, Steven Schveighoffer wrote:

 One further thing: I didn't make the sink version of message  nogc, but 
 in actuality, it could be. Notice how it allocates using the stack. Even 
 if we needed some indeterminate amount of memory, it would be simple to 
 use C malloc/free, or alloca. But traditionally, we don't put any 
 attributes on these base functions. Would it make sense in this case?


Aand, no we can't. Because the sink could actually allocate.

-Steve
Sep 19 2018
parent reply Seb <seb wilzba.ch> writes:
On Wednesday, 19 September 2018 at 21:28:56 UTC, Steven 
Schveighoffer wrote:

 On 9/19/18 5:16 PM, Steven Schveighoffer wrote:

 One further thing: I didn't make the sink version of message 
  nogc, but in actuality, it could be.




We recently introduced support for output ranges in the 
formatting of Phobos:

https://dlang.org/changelog/2.079.0.html#toString

Output ranges have the advantage that they could be  nogc and 
because of the templatization also  safe.
Sep 19 2018
parent Steven Schveighoffer <schveiguy gmail.com> writes:
On 9/19/18 7:53 PM, Seb wrote:

 On Wednesday, 19 September 2018 at 21:28:56 UTC, Steven Schveighoffer 
 wrote:

 On 9/19/18 5:16 PM, Steven Schveighoffer wrote:

 One further thing: I didn't make the sink version of message  nogc, 
 but in actuality, it could be.




 
 We recently introduced support for output ranges in the formatting of 
 Phobos:
 
 https://dlang.org/changelog/2.079.0.html#toString
 
 Output ranges have the advantage that they could be  nogc and because of 
 the templatization also  safe.


I don't think that will work here, as Throwable is a class.

All I can think of is that you would have 2 versions, a  nogc one that 
takes a  nogc delegate, and one that is not.

Of course, your exception type could define something completely 
separate, and you can deal with it in your own project as needed.

If there was a way to say "this is  nogc if you give it a  nogc 
delegate, and not if you don't", that would be useful. The compiler 
could verify it at compile-time.

-Steve
Sep 19 2018
prev sibling next sibling parent reply Atila Neves <atila.neves gmail.com> writes:
On Wednesday, 19 September 2018 at 21:16:00 UTC, Steven 
Schveighoffer wrote:

 Given dip1008, we now can throw exceptions inside  nogc code! 
 This is really cool, and helps make code that uses exceptions 
 or errors  nogc. Except...

 The mechanism to report what actually went wrong for an 
 exception is a string passed to the exception during 
 *construction*. Given that you likely want to make such an 
 exception inside a  nogc function, you are limited to passing a 
 compile-time-generated string (either a literal or one 
 generated via CTFE).


<snip>

I expressed my concern for DIP1008 and the `msg` field when it 
was first announced. I think the fix is easy and a one line 
change to dmd. I also expressed this on that thread but was 
apparently ignored. What's the fix? Have the compiler insert a 
call to the exception's destructor at the end of the `catch(scope 
Exception)` block. That's it. The `msg` field is just a slice, 
point it to RAII managed memory and you're good to go.

Give me deterministic destruction of exceptions caught by scope 
when using dip1008 and I'll give you  nogc exception throwing 
immediately. I've even already written the code!
Sep 20 2018
next sibling parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 9/20/18 6:48 AM, Atila Neves wrote:

 On Wednesday, 19 September 2018 at 21:16:00 UTC, Steven Schveighoffer 
 wrote:

 Given dip1008, we now can throw exceptions inside  nogc code! This is 
 really cool, and helps make code that uses exceptions or errors  nogc. 
 Except...

 The mechanism to report what actually went wrong for an exception is a 
 string passed to the exception during *construction*. Given that you 
 likely want to make such an exception inside a  nogc function, you are 
 limited to passing a compile-time-generated string (either a literal 
 or one generated via CTFE).


 
 <snip>
 
 I expressed my concern for DIP1008 and the `msg` field when it was first 
 announced. I think the fix is easy and a one line change to dmd. I also 
 expressed this on that thread but was apparently ignored. What's the 
 fix? Have the compiler insert a call to the exception's destructor at 
 the end of the `catch(scope Exception)` block. That's it. The `msg` 
 field is just a slice, point it to RAII managed memory and you're good 
 to go.
 
 Give me deterministic destruction of exceptions caught by scope when 
 using dip1008 and I'll give you  nogc exception throwing immediately. 
 I've even already written the code!


I thought it already did that? How is the exception destroyed when 
dip1008 is enabled?

But this means you still have to build msg when throwing the 
error/exception. It's not needed until you print it, and there's no 
reason anyway to make it allocate, even with RAII. For some reason D 
forces msg to be built, but it does't e.g. build the entire stack trace 
string before hand, or build the string that shows the exception class 
name or the file/line beforehand.

-Steve
Sep 20 2018
next sibling parent Atila Neves <atila.neves gmail.com> writes:
On Thursday, 20 September 2018 at 12:48:13 UTC, Steven 
Schveighoffer wrote:

 On 9/20/18 6:48 AM, Atila Neves wrote:

 On Wednesday, 19 September 2018 at 21:16:00 UTC, Steven 
 Schveighoffer wrote:

 [...]


 
 <snip>
 
 I expressed my concern for DIP1008 and the `msg` field when it 
 was first announced. I think the fix is easy and a one line 
 change to dmd. I also expressed this on that thread but was 
 apparently ignored. What's the fix? Have the compiler insert a 
 call to the exception's destructor at the end of the 
 `catch(scope Exception)` block. That's it. The `msg` field is 
 just a slice, point it to RAII managed memory and you're good 
 to go.
 
 Give me deterministic destruction of exceptions caught by 
 scope when using dip1008 and I'll give you  nogc exception 
 throwing immediately. I've even already written the code!


 I thought it already did that? How is the exception destroyed 
 when dip1008 is enabled?


I've had a 2 week holiday since I was working on this so I don't
quite remember. I'll try and find out the specifics for you later.
What I do remember is that the destructor didn't get called.


 But this means you still have to build msg when throwing the 
 error/exception. It's not needed until you print it, and 
 there's no reason anyway to make it allocate, even with RAII.


Maybe, but it's a lot easier to just allocate and have the 
destructor take care of it.


 For some reason D forces msg to be built, but it does't e.g. 
 build the entire stack trace string before hand, or build the 
 string that shows the exception class name or the file/line 
 beforehand.


Good points.
Sep 20 2018
prev sibling next sibling parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Thu, Sep 20, 2018 at 08:48:13AM -0400, Steven Schveighoffer via
Digitalmars-d wrote:
[...]

 But this means you still have to build msg when throwing the
 error/exception. It's not needed until you print it, and there's no
 reason anyway to make it allocate, even with RAII. For some reason D
 forces msg to be built, but it does't e.g. build the entire stack
 trace string before hand, or build the string that shows the exception
 class name or the file/line beforehand.


[...]

IIRC, originally the stacktrace was also built at exception construction
time. But it was causing a major performance hit, so eventually someone
changed the code to construct it lazily (i.e., only when the catcher
actually tries to look it up).

I think it makes sense to also make .msg lazy, if the exception object
is already carrying enough info to build the message when the catcher
asks for it. And if the catcher doesn't ask for it, we saved an extra GC
allocation (which is a plus even if we're not trying to go  nogc).


T

-- 
Computers shouldn't beep through the keyhole.
Sep 20 2018
parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 9/20/18 11:06 AM, H. S. Teoh wrote:

 On Thu, Sep 20, 2018 at 08:48:13AM -0400, Steven Schveighoffer via
Digitalmars-d wrote:
 [...]

 But this means you still have to build msg when throwing the
 error/exception. It's not needed until you print it, and there's no
 reason anyway to make it allocate, even with RAII. For some reason D
 forces msg to be built, but it does't e.g. build the entire stack
 trace string before hand, or build the string that shows the exception
 class name or the file/line beforehand.


 [...]
 
 IIRC, originally the stacktrace was also built at exception construction
 time. But it was causing a major performance hit, so eventually someone
 changed the code to construct it lazily (i.e., only when the catcher
 actually tries to look it up).
 
 I think it makes sense to also make .msg lazy, if the exception object
 is already carrying enough info to build the message when the catcher
 asks for it. And if the catcher doesn't ask for it, we saved an extra GC
 allocation (which is a plus even if we're not trying to go  nogc).


Except we DON'T construct the stack trace string, even lazily. If you 
look at the code I posted, it's output directly to the output buffer 
(via the sink delegate), without ever having allocated.

I think we can do that for the message too (why not, it's all 
supported). But either one (using GC at print time, or lazily outputting 
to buffer at print time) solves the biggest problem -- being able to 
construct an exception without the GC.

Plus, this nudges developers of exceptions to store more useful data. If 
you catch an exception that has details in it, possibly it is only going 
to be in the string, which you now have to *parse* to get out what the 
problem was. If instead it was standard practice just to store the 
details, and then construct the string later, more useful information 
would be available in the form of fields/accessors.

Think about this -- every ErrnoException that is thrown allocates its 
message via the GC on construction. Even if you catch that and just look 
at the errno code. Even with dip1008:

https://github.com/dlang/phobos/blob/6a15dfbe18f9151379f6337f53a3c41d12dee939/std/exception.d#L1625

-Steve
Sep 20 2018
parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Thu, Sep 20, 2018 at 11:25:27AM -0400, Steven Schveighoffer via
Digitalmars-d wrote:

 On 9/20/18 11:06 AM, H. S. Teoh wrote:


[...]

 IIRC, originally the stacktrace was also built at exception
 construction time. But it was causing a major performance hit, so
 eventually someone changed the code to construct it lazily (i.e.,
 only when the catcher actually tries to look it up).
 
 I think it makes sense to also make .msg lazy, if the exception
 object is already carrying enough info to build the message when the
 catcher asks for it. And if the catcher doesn't ask for it, we saved
 an extra GC allocation (which is a plus even if we're not trying to
 go  nogc).


 
 Except we DON'T construct the stack trace string, even lazily. If you
 look at the code I posted, it's output directly to the output buffer
 (via the sink delegate), without ever having allocated.
 
 I think we can do that for the message too (why not, it's all
 supported).  But either one (using GC at print time, or lazily
 outputting to buffer at print time) solves the biggest problem --
 being able to construct an exception without the GC.


[...]

Yeah, that's what I meant. :D  Well, for backward compatibility we could
still have .msg allocate and return a string, but we could provide an
overload / alternate member function that writes directly to a sink
instead.  Then we don't ever have to allocate unless the sink itself
does.


T

-- 
I am a consultant. My job is to make your job redundant. -- Mr Tom
Sep 20 2018
parent Neia Neutuladh <neia ikeran.org> writes:
On Thursday, 20 September 2018 at 15:52:15 UTC, H. S. Teoh wrote:

 Yeah, that's what I meant. :D  Well, for backward compatibility 
 we could still have .msg allocate and return a string, but we 
 could provide an overload / alternate member function that 
 writes directly to a sink instead.  Then we don't ever have to 
 allocate unless the sink itself does.


I believe Tango did this a decade ago. It's a solid strategy. 
However, with Tango, the default was to implement the 
toString(sink) function by calling the regular toString(), which 
was quite convenient but won't work with  nogc.
Sep 20 2018
prev sibling parent reply Atila Neves <atila.neves gmail.com> writes:
On Thursday, 20 September 2018 at 12:48:13 UTC, Steven 
Schveighoffer wrote:

 On 9/20/18 6:48 AM, Atila Neves wrote:

 On Wednesday, 19 September 2018 at 21:16:00 UTC, Steven 
 Schveighoffer wrote:

 Given dip1008, we now can throw exceptions inside  nogc code! 
 This is really cool, and helps make code that uses exceptions 
 or errors  nogc. Except...

 The mechanism to report what actually went wrong for an 
 exception is a string passed to the exception during 
 *construction*. Given that you likely want to make such an 
 exception inside a  nogc function, you are limited to passing 
 a compile-time-generated string (either a literal or one 
 generated via CTFE).


 
 <snip>
 
 I expressed my concern for DIP1008 and the `msg` field when it 
 was first announced. I think the fix is easy and a one line 
 change to dmd. I also expressed this on that thread but was 
 apparently ignored. What's the fix? Have the compiler insert a 
 call to the exception's destructor at the end of the 
 `catch(scope Exception)` block. That's it. The `msg` field is 
 just a slice, point it to RAII managed memory and you're good 
 to go.
 
 Give me deterministic destruction of exceptions caught by 
 scope when using dip1008 and I'll give you  nogc exception 
 throwing immediately. I've even already written the code!


 I thought it already did that?


Nope:

---------------
class MyException: Exception {
     static int numInstances;
     this(string msg) {
         super(msg);
         ++numInstances;
     }

     ~this() {
         --numInstances;
     }
}

void main() {
     assert(MyException.numInstances == 0);

     try
         throw new MyException("oops");
     catch(MyException _)
         assert(MyException.numInstances == 1);

     assert(MyException.numInstances == 0);
}
---------------

% dmd -dip1008 -run exception.d
core.exception.AssertError exception.d(21): Assertion failure


 How is the exception destroyed when dip1008 is enabled?


Apparently, it isn't. Which renders dip1008 pretty much useless 
since we could already use static immutable exceptions before.


 But this means you still have to build msg when throwing the 
 error/exception. It's not needed until you print it, and 
 there's no reason anyway to make it allocate, even with RAII. 
 For some reason D forces msg to be built, but it does't e.g. 
 build the entire stack trace string before hand, or build the 
 string that shows the exception class name or the file/line 
 beforehand.



Allocating and building the string doesn't bother me - in all of 
my uses it's eventually going to get printed (which means the 
string needed to be built), and the exceptional path can be slow, 
I don't mind.

But, one could always store a tuple of members in an exception 
class instead and only build the string on demand.

I just think it's easier with an RAII string.
Oct 19 2018
parent reply Nicholas Wilson <iamthewilsonator hotmail.com> writes:
On Friday, 19 October 2018 at 23:34:01 UTC, Atila Neves wrote:

 On Thursday, 20 September 2018 at 12:48:13 UTC, Steven 
 Schveighoffer wrote:

 How is the exception destroyed when dip1008 is enabled?


 Apparently, it isn't. Which renders dip1008 pretty much useless 
 since we could already use static immutable exceptions before.


Wow, that is useless! Is that an implementation bug or was that 
specified by the DIP?
Oct 19 2018
parent Atila Neves <atila.neves gmail.com> writes:
On Friday, 19 October 2018 at 23:46:29 UTC, Nicholas Wilson wrote:

 On Friday, 19 October 2018 at 23:34:01 UTC, Atila Neves wrote:

 On Thursday, 20 September 2018 at 12:48:13 UTC, Steven 
 Schveighoffer wrote:

 How is the exception destroyed when dip1008 is enabled?


 Apparently, it isn't. Which renders dip1008 pretty much 
 useless since we could already use static immutable exceptions 
 before.


 Wow, that is useless! Is that an implementation bug or was that 
 specified by the DIP?


I hope it's a bug - I opened an issue:

https://issues.dlang.org/show_bug.cgi?id=19317
Oct 19 2018
prev sibling parent welkam <wwwelkam gmail.com> writes:
On Thursday, 20 September 2018 at 10:48:35 UTC, Atila Neves wrote:

 What's the fix? Have the compiler insert a call to the 
 exception's destructor at the end of the `catch(scope 
 Exception)` block.


If I understood you correctly then we have same idea. If 
exception is not handled or re thrown then scope "exits" trough 
scope(failure) in which case no deallocation should be performed 
and only if scope "exits" trough scope(success), meaning 
exception/s were handled, destructors need to be called.

To make exceptions nogc we need destructors that are aware of how 
scope was exited like:

~ this(exit) ( ) {}
~ this(success) ( ) {}
~ this(failure) ( ) {}

With this its easy to make all exceptions nogc
Oct 21 2018
prev sibling next sibling parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Wednesday, 19 September 2018 at 21:16:00 UTC, Steven 
Schveighoffer wrote:

 As Andrei says -- Destroy!


Nah, I agree. Actually, I'm of the opinion that string error 
messages in exceptions ought to be considered harmful: you 
shouldn't be doing strings at all. All the useful information 
should be in the type - the class name and the members with 
details.

Well, defining a new class can sometimes be a mild hassle... but 
for really common ones, we really should just do it, and other 
ones can be done as templated classes or templated factory 
functions that define a new class right there and then.

http://arsdnet.net/dcode/exception.d

That's the proof-of-concept I wrote for this years ago, go to the 
bottom of the file for the usage example. It uses a reflection 
mixin to make writing the new classes easy, and I even wrote an 
enforce thing that can add more info by creating a subclass that 
stores arguments to functions so it can print it all (assuming 
they are sane to copy like strings or value things lol)

	enforce!fopen("nofile.txt".ptr, "rb".ptr);

MyExceptionBase exception.d(38): fopen call failed
         filename = nofile.txt
         mode = rb
----------------
Sep 20 2018
parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 9/20/18 11:33 AM, Adam D. Ruppe wrote:

 On Wednesday, 19 September 2018 at 21:16:00 UTC, Steven Schveighoffer 
 wrote:

 As Andrei says -- Destroy!


 
 Nah, I agree. Actually, I'm of the opinion that string error messages in 
 exceptions ought to be considered harmful: you shouldn't be doing 
 strings at all. All the useful information should be in the type - the 
 class name and the members with details.
 
 Well, defining a new class can sometimes be a mild hassle... but for 
 really common ones, we really should just do it, and other ones can be 
 done as templated classes or templated factory functions that define a 
 new class right there and then.
 
 http://arsdnet.net/dcode/exception.d
 
 That's the proof-of-concept I wrote for this years ago, go to the bottom 
 of the file for the usage example. It uses a reflection mixin to make 
 writing the new classes easy, and I even wrote an enforce thing that can 
 add more info by creating a subclass that stores arguments to functions 
 so it can print it all (assuming they are sane to copy like strings or 
 value things lol)
 
      enforce!fopen("nofile.txt".ptr, "rb".ptr);
 
 MyExceptionBase exception.d(38): fopen call failed
          filename = nofile.txt
          mode = rb
 ----------------



Awesome! This is just what I was thinking of. In fact, I did something 
similar locally since I needed to know what the slice parameters that 
were failing were. I still had to trick the  nogc to get around the "new 
Exception" piece.

The printMembers thing is nice. I think for druntime/phobos, however, we 
should have a base that just calls a virtual function with the idea that 
the message is printed, and then a further-derived type could do the 
printMembers thing if that's what you want.

-Steve
Sep 20 2018
parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Thursday, 20 September 2018 at 15:52:03 UTC, Steven 
Schveighoffer wrote:

 I needed to know what the slice parameters that were failing 
 were.


Aye. Note that RangeError is called by the compiler though, so 
you gotta patch dmd to make it pass the arguments to it for 
index. Ugh. I did a PR for this once but it got shot down because 
of an allegeded (without evidence btw) performance degradation. 
Ugh.


 The printMembers thing is nice. I think for druntime/phobos, 
 however, we should have a base that just calls a virtual 
 function with the idea that the message is printed


Aye.
Sep 20 2018
parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 9/20/18 12:24 PM, Adam D. Ruppe wrote:

 On Thursday, 20 September 2018 at 15:52:03 UTC, Steven Schveighoffer wrote:

 I needed to know what the slice parameters that were failing were.


 
 Aye. Note that RangeError is called by the compiler though, so you gotta 
 patch dmd to make it pass the arguments to it for index. Ugh. I did a PR 
 for this once but it got shot down because of an allegeded (without 
 evidence btw) performance degradation. Ugh.


Well, you can always override that. Just do the check yourself and throw 
the error you want ;)

In my case, that's what I did anyway.

I don't know how a performance problem can occur on an error being 
thrown anyway -- the process is about to end.

-Steve
Sep 20 2018
next sibling parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Thursday, 20 September 2018 at 17:14:12 UTC, Steven 
Schveighoffer wrote:

 I don't know how a performance problem can occur on an error 
 being thrown anyway -- the process is about to end.


Walter's objection was code size - it would throw stuff out of 
cache lines, even if it doesn't need to actually run.

So like this line:

int[] a;
a = a[1 .. $];

With no bounds checking is just

inc a.ptr;

but with bounds checking it becomes something more like

mov ecx, a.length
cmp ecx, 1 // in other words, if length >= offset
jae proceed
push line
push file
call d_arraybounds // throws the error
proceed:
inc a.ptr


Now, what my patch did was just, right before push line, it 
inserted "push length; push offset;". I believe this to be 
trivial since they are already loaded in registers or immediates 
and thus just a couple bytes for those instructions, but Walter 
(as I recall, this was a while ago and I didn't look up his exact 
words when writing this) said even a couple bytes are important 
for such a common operation as it throws off the L1 caches. I 
never got around to actually measuring the performance impact to 
prove one way or another.

But... even if that is a problem, dmd -O will usually rearrange 
that to avoid the jump on the in-bounds case, and I'm sure 
ldc/gdc do too, sooooo the extra pushes' instruction bytes are 
off the main execution path anyway and thus shouldn't waste cache 
space.


idk though. regardless, to me, the extra info is *well* worth the 
cost anyway.
Sep 20 2018
parent Steven Schveighoffer <schveiguy gmail.com> writes:
On 9/20/18 1:58 PM, Adam D. Ruppe wrote:

 On Thursday, 20 September 2018 at 17:14:12 UTC, Steven Schveighoffer wrote:

 I don't know how a performance problem can occur on an error being 
 thrown anyway -- the process is about to end.


 
 Walter's objection was code size - it would throw stuff out of cache 
 lines, even if it doesn't need to actually run.
 
 So like this line:
 
 int[] a;
 a = a[1 .. $];
 
 With no bounds checking is just
 
 inc a.ptr;
 
 but with bounds checking it becomes something more like
 
 mov ecx, a.length
 cmp ecx, 1 // in other words, if length >= offset
 jae proceed
 push line
 push file
 call d_arraybounds // throws the error
 proceed:
 inc a.ptr
 
 
 Now, what my patch did was just, right before push line, it inserted 
 "push length; push offset;". I believe this to be trivial since they are 
 already loaded in registers or immediates and thus just a couple bytes 
 for those instructions, but Walter (as I recall, this was a while ago 
 and I didn't look up his exact words when writing this) said even a 
 couple bytes are important for such a common operation as it throws off 
 the L1 caches. I never got around to actually measuring the performance 
 impact to prove one way or another.
 
 But... even if that is a problem, dmd -O will usually rearrange that to 
 avoid the jump on the in-bounds case, and I'm sure ldc/gdc do too, 
 sooooo the extra pushes' instruction bytes are off the main execution 
 path anyway and thus shouldn't waste cache space.
 
 
 idk though. regardless, to me, the extra info is *well* worth the cost 
 anyway.


Sounds like a case of premature optimization at best.

Besides, if it is a performance issue, you aren't doing bounds checks on 
every slice/index anyway. I know in iopipe, to squeeze out every bit of 
performance, I avoid bounds checks when I know from previous asserts the 
bounds are correct.

-Steve
Sep 20 2018
prev sibling parent reply Paolo Invernizzi <paolo.invernizzi gmail.com> writes:
On Thursday, 20 September 2018 at 17:14:12 UTC, Steven 
Schveighoffer wrote:

 On 9/20/18 12:24 PM, Adam D. Ruppe wrote:

 On Thursday, 20 September 2018 at 15:52:03 UTC, Steven 
 Schveighoffer wrote:

 I needed to know what the slice parameters that were failing 
 were.


 
 Aye. Note that RangeError is called by the compiler though, so 
 you gotta patch dmd to make it pass the arguments to it for 
 index. Ugh. I did a PR for this once but it got shot down 
 because of an allegeded (without evidence btw) performance 
 degradation. Ugh.


 Well, you can always override that. Just do the check yourself 
 and throw the error you want ;)

 In my case, that's what I did anyway.

 I don't know how a performance problem can occur on an error 
 being thrown anyway -- the process is about to end.

 -Steve


If ` nogc` could be relaxed for `new Error` exactly for that 
reason, pieces of Phobos could be turned ` nogc`...

But I admit that that change would be controversial...

- Paolo
Oct 20 2018
parent reply Mike Parker <aldacron gmail.com> writes:
On Saturday, 20 October 2018 at 13:48:32 UTC, Paolo Invernizzi 
wrote:


 If ` nogc` could be relaxed for `new Error` exactly for that 
 reason, pieces of Phobos could be turned ` nogc`...

 But I admit that that change would be controversial...


https://github.com/dlang/DIPs/blob/master/DIPs/DIP1008.md
Oct 20 2018
parent Paolo Invernizzi <paolo.invernizzi gmail.com> writes:
On Saturday, 20 October 2018 at 14:56:37 UTC, Mike Parker wrote:

 On Saturday, 20 October 2018 at 13:48:32 UTC, Paolo Invernizzi 
 wrote:


 If ` nogc` could be relaxed for `new Error` exactly for that 
 reason, pieces of Phobos could be turned ` nogc`...

 But I admit that that change would be controversial...


 https://github.com/dlang/DIPs/blob/master/DIPs/DIP1008.md


Yep, you are right, of course...  8-/

/P
Oct 20 2018
prev sibling parent reply Jonathan M Davis <newsgroup.d jmdavisprog.com> writes:
On Wednesday, September 19, 2018 3:16:00 PM MDT Steven Schveighoffer via 
Digitalmars-d wrote:

 Given dip1008, we now can throw exceptions inside  nogc code! This is
 really cool, and helps make code that uses exceptions or errors  nogc.
 Except...


I pointed out this problem when the DIP was orginally proposed and have
always thought that it was kind of pointless because of this issue. The core
problem is that Exception predates  nogc. It predates ranges. Its design
makes sense if exceptions are rare, and you're willing to use the GC. IMHO,
it even makes sense in most cases with most code that is trying to avoid
allocations, because the allocations are only going to occur when an error
condition occurs. So, for most code bases, they won't affect performance.
The problem is that even though that usually makes perfect sense, if you've
written your code so that the non-error path doesn't allocate, you'd really
like to be able to mark it with  nogc, and you can't because of the
exceptions. The DIP helps but not enough to really matter. And of course, in
those code bases where you actually can't afford to have the error path
allocate, it's even more of a problem.

So, what we have is an Exception class that works perfectly well in a world
without  nogc but which doesn't work with  nogc worth anything. And if we
want to fix that, I think that we need to figure out how to fix Exception in
a way that we can sanely transitition to whatever the new way we handle
exception messages would be. I think that the message member was added by
Dicebot as an attempt to fix this issue, because Sociomantic needed it, but
I don't know exactly what's going on with that or  __future.

- Jonathan M Davis
Sep 21 2018
parent reply Nemanja Boric <dlang nemanjaboric.com> writes:
On Friday, 21 September 2018 at 09:10:06 UTC, Jonathan M Davis 
wrote:

 [...]
 I think that the message member was added by Dicebot as an 
 attempt to fix this issue, because Sociomantic needed it, but I 
 don't know exactly what's going on with that or  __future.

 - Jonathan M Davis


The  __future is fully (to a reasonable degree) implemented - and 
the `Throwable.message` was marked with this attribute to prevent 
breaking the user code when introducing this field, and it 
probably can just be removed from there at this point (since many 
releases have been released).

This is the PR when Throwable.message was first introduced: 
https://github.com/dlang/druntime/pull/1445 We had the same 
discussion there, and you can see sociomantic's use case in 
Mihail's comments there. The entire PR have a very good 
discussion and we (not Sociomantic) still need sink-based method 
IIUC.
Sep 21 2018
parent reply Nemanja Boric <dlang nemanjaboric.com> writes:
On Friday, 21 September 2018 at 10:06:06 UTC, Nemanja Boric wrote:

 On Friday, 21 September 2018 at 09:10:06 UTC, Jonathan M Davis 
 wrote:

 [...]


 The  __future is fully (to a reasonable degree) implemented - 
 and the `Throwable.message` was marked with this attribute to 
 prevent breaking the user code when introducing this field, and 
 it probably can just be removed from there at this point (since 
 many releases have been released).

 [...]


Interesting quote from Martin on that PR:


 With regards to Throwable.message we agreed on making it one of 
 the first users of an upcoming reference counted string. 
 Details on the design of the reference counted string first to 
 be discussed on dlang-study.
Sep 21 2018
parent Petar Kirov [ZombineDev] <petar.p.kirov gmail.com> writes:
On Friday, 21 September 2018 at 11:48:50 UTC, Nemanja Boric wrote:

 On Friday, 21 September 2018 at 10:06:06 UTC, Nemanja Boric 
 wrote:

 On Friday, 21 September 2018 at 09:10:06 UTC, Jonathan M Davis 
 wrote:

 [...]


 The  __future is fully (to a reasonable degree) implemented - 
 and the `Throwable.message` was marked with this attribute to 
 prevent breaking the user code when introducing this field, 
 and it probably can just be removed from there at this point 
 (since many releases have been released).

 [...]


 Interesting quote from Martin on that PR:


 With regards to Throwable.message we agreed on making it one 
 of the first users of an upcoming reference counted string. 
 Details on the design of the reference counted string first to 
 be discussed on dlang-study.




If in theory D had all the memory-safety features to make this 
work (think what languages like Rust have with linear/affine 
types [0] [1]) along with the necessary library implementations 
of smart pointers, RC-aware slicing, etc..., I would still prefer 
the sink-based approach in this situation, as it's much more 
elegant in my opinion.

[0]: http://wiki.c2.com/?LinearTypes
[1]: https://www.tweag.io/posts/2017-03-13-linear-types.html
Sep 21 2018