Create a LazyString type, for places (like errors) which we would prefer to defer the actual work

[Keno]

One common theme when looking at inference logs is that formatting error messages
tends to pull in the whole I/O and printing system which a) can take some time to
infer and b) adds backedges if that stuff gets invalidated. Both seem wasteful.
This is an attempt to try to improve the situation. For various error structs
that used to contain a string, they now take either a String or a closure that
will format the required string on demand when showing the error.

I have some tooling to count the total number of methods that inference looks
at and went through and switched over all the error messages that are used
by *(::Matrix{Float64}, ::Matrix{Float64} to the new setup. Doing this
reduces the total number of methods inference looks at by about 20%,
which seems like a worthwhile improvement.

Obviously there are a lot of error sites that could potentially throw, but this approach is backwards compatible, so if we like the direction, we can merge it and then we can gradually go through and clean up the callsite (or somebody could write a femtocleaner pass to do it).

[JeffBezanson]

This is not my favorite way to deal with this, since the large number of types generated for the closures is also pretty sub-optimal for the compiler. A possible alternative is to have a macro that makes string interpolation lazy, i.e. returning an object with the arguments. That might be useful for other purposes as well.

[Keno]

Alright, how do you like this version? Seems to still give the same benefits in inference (modulo a few extra LazyString constructors).

[Keno]

@JeffBezanson do you like this approach better? If so, I'll fix up this PR and get it ready.

[StefanKarpinski]

I kind of like this as an implementation, but it suggests to me that having a macro for throwing errors would be beneficial, e.g. @throw DomainError "blah $x blah".

[JeffBezanson]

Suggested change

	function show(io::IO, s::LazyString)
	function print(io::IO, s::LazyString)

[JeffBezanson]

Yes I like this. Unfortunately implementing the normal string interface for this type is not very natural, but oh well.

[c42f]

@StefanKarpinski related to a possible @throw, I've sometimes thought it would be useful to be able to attach arbitrary key-value pairs to exceptions in much the same way we attach them to log records. I think people sometimes have a temptation to log-then-throw for adding context which is really not ideal.

(See also this crazy idea #7026 (comment) on how we might unify return codes with exceptions. It would make even more sense if combined with pattern matching of key value pairs.)

[StefanKarpinski]

I've been using the logging infrastructure for PkgServer and it's quite nice to just list key-value pairs for values that may be of interest when looking at logs. Doing something similar for errors would make sense to me as well.

[User-764Q]

This has been open for a while, is it worth closing it now?

[KristofferC]

@User-764Q please stop commenting on issues/PRs when the only information you bring up is that they are "old". It creates notifications for a lot of people.

[Roger-luo]

Just saw this issue it seems this is also useful for solving KristofferC/Crayons.jl#47 since when interpolating crayons we are lack of the IO context. Doing the interpolation lazily would defer this information when we actually print it which solves the problem.

I'm wondering if people still want it in Base for other reasons or maybe we can have this in a package first.

[Keno]

I think this is good to go. Will merge in a bit so if there are any objections, say so now.

[c42f]

I think this is good useful utility, and I like the @lazy_str export. For minimalism I could live without the LazyString export, but leaving it in also seems harmless enough.

[c42f]

Should these cases be using the string macro for consistency? It seems you've included strings/lazy.jl before abstractarray.jl during bootstrap so I guess it should work?

[edit] ah, I see that some uses of string() are just for keeping indentation under control. It sure would be nice if it was easier to reuse the parsing logic from #40753 so we had consistency of escaping rules for these kind of "almost but not quite a normal string" type of custom string literals. Rather than needing the raw string escaping rules.

[Keno]

I just did the straightforward replacement. We can change these for consistency in a followup.

[c42f]

Certainly fine by me.

[vtjnash]

This seems like it might potentially move in the opposite direction of the recent escape analysis-driven design changes, where we wanted to capture objects less frequently.

[vtjnash]

It might be better to allocate an Array here, so we do not need to create a whole new Tuple type and can examine and iterate it more easily later?

[Keno]

It can be be an immutable array once that's merged, but currently the analysis in #43852 is not strong enough to model the mutation of the mutable array at the moment. I think for the current use cases, a tuple is mostly fine, since the types will be known at compile time and are stable. Plus it's the error path anyway, which we heavily pessimize and in particular, don't infer, so it'll allocate tons of tuple types anyway.

[vtjnash]

It feels a bit cheeky to declare that this is a kind of String, then implement none of the interface for one.

We could potentially make this object mutable, then swap the field for the string'd value when you first access it, perhaps?

[Keno]

Yes, I think that's reasonable. Let me do that.

[Keno]

This seems like it might potentially move in the opposite direction of the recent escape analysis-driven design changes, where we wanted to capture objects less frequently.

Well, as I said in those discussions, I don't like the idea to string summarize on error construction and if there are escaping issues, we should insert maybecopies.

[KristofferC]

Is there any worry about race conditions for code like this?

[vtjnash]

Yes. We can use an atomic acquire and cmpswap release here to fix that. (Or perhaps create an "assign_once" type, that manages this pattern for us?)

[aviatesk]

This seems like it might potentially move in the opposite direction of the recent escape analysis-driven design changes, where we wanted to capture objects less frequently.

Well, as I said in those discussions, I don't like the idea to string summarize on error construction and if there are escaping issues, we should insert maybecopies.

I'm still not convinced that maybecopy is the right solution. One problem of the maybecopy idea is that it will require a global transformation, or many similar builtin function definitions.
E.g. if we want to allocate a on the stack in the following example, where a can only escape via possible bounds error in arrayref call, we need to change %24 = Base.arrayref(true, %2, 2)::Int64 to something like %24 = Base.arrayref_copy_on_throw(true, %2, 2)::Int, where arrayref_copy_on_throw uses another version of BoundsError constructor that makes a copy of a:

julia> const Lx = Ref{Any}();

julia> @noinline storelen!(xs) = Lx[] = length(xs);

julia> code_escapes() do
          a = [1,2,3]
          storelen!(a)
          a[2]
      end
#9() in Main at REPL[9]:2
2 ◌  1 ─ %1  = Core.tuple(1, 2, 3)::Core.Const((1, 2, 3))                                │╻      vect
  ↑  │   %2  = $(Expr(:foreigncall, :(:jl_alloc_array_1d), Vector{Int64}, svec(Any, Int64), 0, :(:ccall), Vector{Int64}, 3, 3))::Vector{Int64}
  ◌  └──       goto #7 if not true                                                       ││     
  ✓  2 ┄ %4  = φ (#1 => 1, #6 => %16)::Int64                                             ││     
  ✓  │   %5  = φ (#1 => 1, #6 => %17)::Int64                                             ││     
  ✓  │   %6  = φ (#1 => 1, #6 => %9)::Int64                                              ││     
  ◌  │   %7  = Base.getfield(%1, %4, true)::Int64                                        ││╻      getindex
  ◌  │         Base.arrayset(false, %2, %7, %6)::Vector{Int64}                           ││╻      setindex!
  ◌  │   %9  = Base.add_int(%6, 1)::Int64                                                ││     
  ◌  │   %10 = (%5 === 3)::Bool                                                          │││╻      ==
  ◌  └──       goto #4 if not %10                                                        │││    
  ◌  3 ─       Base.nothing::Nothing                                                     │││    
  ◌  └──       goto #5                                                                   │││    
  ◌  4 ─ %14 = Base.add_int(%5, 1)::Int64                                                │││╻      +
  ◌  └──       goto #5                                                                   ││╻      iterate
  ✓  5 ┄ %16 = φ (#4 => %14)::Int64                                                      ││     
  ✓  │   %17 = φ (#4 => %14)::Int64                                                      ││     
  ◌  │   %18 = φ (#3 => true, #4 => false)::Bool                                         ││     
  ◌  │   %19 = Base.not_int(%18)::Bool                                                   ││     
  ◌  └──       goto #7 if not %19                                                        ││     
  ◌  6 ─       goto #2                                                                   ││     
  ◌  7 ┄       goto #8                                                                   ││     
3 ◌  8 ─       invoke Main.storelen!(%2::Vector{Int64})::Any                             │      
4 ◌  │   %24 = Base.arrayref(true, %2, 2)::Int64                                         │╻      getindex
  ◌  └──       return %24                                                                │

This is because the constructor of BoundsError is opaque from this frame, and it's tricky to change the semantics of it based on the analysis on this frame. So probably the most sensible way to implement the maybecopy idea is to define "no-capturing on exception" alternatives of certain builtin functions whose no-throw checks inherently need to be conservative.

To me that sounds more complicated and I'd say it's much simpler if we change the semantics of certain error constructors so that they don't capture and let analyzers and optimizers assume that invariant.

[Keno]

Yes, the initial implementation of maybecopy is just to always copy the array, so you can assume no-escape. The issue people had with that is that the array may be big, so we want to have the flexibility to not do that in contexts where memory allocations are supposed to be tightly controlled. That's why the maybecopy is useful, because the optimizer can choose. I agree with you of course that it requires a global transformation, but I also think the extra memory issue is somewhat theoretical, and by the time it becomes a problem we should be able to do the compiler transform.