Should DomainErrors accept arguments?

[kshyatt]

Re: #12121's discussion for @tkelman.

Right now, DomainError doesn't take any arguments and will throw a MethodError if you try to pass an informative string. This is important because it's used in many performance critical math functions like sqrt, cf. @andreasnoack. @simonster chimed in to point out that

We could have DomainError{T<:Union(None,ByteString)} where DomainError() would be a DomainError{None}. I don't think that would be any more expensive when there's no string to be thrown. Would need some changes on the codegen side, though...

and @yuyichao mentioned the performance regression discussed at #11508. @simonster pointed out that

I'm not sure removing the GC frame would be enough. It would eliminate the additional cost for a DomainError with a string parameter in code that doesn't throw a DomainError, but you'd still have additional overhead if you actually throw a DomainError, which might matter.

What do we want to do going forward? Should we fix codegen, use #11508, or something else?

[yuyichao]

but you'd still have additional overhead if you actually throw a DomainError, which might matter.

Somehow I missed this comment.

Yes, there might be a very small additional overhead but it doesn't not matter much since it's tiny compare to other overhead we pay when throwing an error.

I also think that even if we have much cheaper exception handling (which requires a looot of work AFAICT) the additional debug info provided with the exception should still be more useful than the small overhead of allocating a exception. (ok, this part is irrelevant for this issue.)

[simonster]

@yuyichao It seems you're right. I did not realize how expensive it was to throw an exception. Given the cost I agree it's not worth worrying about the allocation overhead.

[yuyichao]

@simonster Yeah I was a little shocked when I did the benchmark a while ago although it's kind of expected given how much we do before throwing the error.....

Given #11508 cannot be merged/rebased until @vtjnash finishes the codegen refactoring and that it does not make throwing error 100% free for the normal path yet, the current strategy AFAICT is to use a specialized and non-inlined error thrower for the case where the function will not have GC root otherwise. (See @mbauman 's bound checking functions.)

[jiahao]

Ref: #9693

[KristofferC]

nextprod could use this, ref #20410

[oxinabox]

Is there an issue for the performance problems that are blocking this?
It seems like a pretty important feature of DomainError.
Otherwise it tells you that you are wrong, but not why you were wrong.

[timholy]

#18521. Fixing this is quite a pileup of issues:

• fix started in Make InexactError more informative #20005
• a more straightforward approach started in Replace several intrinsics with Julia equivalents #22202
• but this can't be merged until we have Decide inline-worthiness based on a more nuanced cost model #22210
• which is currently waiting on The GC root placement pass 1.0 deserves #21888 (not to mention my travel schedule)