Rewrite printf functionality

[quinnj]

Based on #32799.

The main commits unique here rewrite the internal base/printf.jl functionality for about half the code, with 2-3x perf improvements. It utilizes the new ryu float printing mechanics in #32799. These commits also remove all the grisu code.

Status: this is pretty far along, I haven't done the %a/%A type specifier yet, or made it work for BigFloat either, and there's still a good chunk of tests we need, but it's pretty useable and worth starting to get reviews on.

[StefanKarpinski]

You may already have fixed this, but note #16977.

[StefanKarpinski]

This code is so impressively bonkers 😂. Thank you for doing this, @quinnj!

[musm]

wow great work. Does this make it any easier to implement a non-macro printf function ?

[quinnj]

@StefanKarpinski, I agree :). It's interesting to go back and look at all this code that was written so early on, like a peek into an "old world" of sorts. I think it's a big testament to how much the compiler and code generation "mental model" has improved: I think reaching for meta-programming used to be so much more necessary in order to hand-craft code, whereas today you can rely so much on the compiler that you can just write the code much more "naturally" and everything just works.

@musm, yes, the approach in this PR basically follows what we do for Dates formatting or Regex: i.e. you can create a "static" Printf.Format(str) Format object, with a corresponding Printf.format"%d" macro string to create a Format at parse-time. We then have a Printf.format(io, f::Format, args...) method that applies the formatting of f::Format to args.... So the @printf macro is really provided here for backwards compatibility, since the definition is essentially just:

macro printf(io, f, args...)
    fmt = Printf.Format(f)
    return esc(:(Printf.format($io, $fmt, $(args...)))
end

[KristofferC]

Why disable all precompile in the printf stdlib?

[quinnj]

Ah, I was going to mention it to you, it seems we run into an unfortunate bug w/ how literally we're generating the precompile statements:

Generating precompile statements...┌ Error: Failed to precompile precompile(Tuple{typeof(Base.Printf.format), Array{UInt8, 1}, Int64, Base.Printf.Format{Base.CodeUnits{UInt8, String}, Tuple{Base.Printf.Spec{Base.Val{Char(0x64000000)}}, Base.Printf.Spec{Base.Val{Char(0x73000000)}}, Base.Printf.Spec{Base.Val{Char(0x73000000)}}}}, Float64, String, String})
└ @ Main.anonymous /Users/jacobquinn/juliasrc/contrib/generate_precompile.jl:167
ERROR: LoadError: LoadError: Base.CodePointError{UInt32}(0x64000000)
Stacktrace:
 [1] code_point_err(::UInt32) at ./char.jl:86
 [2] Char(::UInt32) at ./char.jl:160
 [3] top-level scope at string:1
 [4] include_string(::Module, ::String, ::String) at ./loading.jl:1064
 [5] include_string at ./loading.jl:1068 [inlined]
 [6] macro expansion at /Users/jacobquinn/juliasrc/contrib/generate_precompile.jl:165 [inlined]
 [7] macro expansion at ./util.jl:212 [inlined]
 [8] (::getfield(Main.anonymous, Symbol("##2#6")){Float64,Base.DevNull})(::String, ::IOStream) at /Users/jacobquinn/juliasrc/contrib/generate_precompile.jl:159
 [9] mktemp(::getfield(Main.anonymous, Symbol("##2#6")){Float64,Base.DevNull}, ::String) at ./file.jl:577
 [10] mktemp at ./file.jl:575 [inlined]
 [11] generate_precompile_statements() at /Users/jacobquinn/juliasrc/contrib/generate_precompile.jl:70
 [12] top-level scope at /Users/jacobquinn/juliasrc/contrib/generate_precompile.jl:180
in expression starting at string:1
in expression starting at /Users/jacobquinn/juliasrc/contrib/generate_precompile.jl:8
*** This error is usually fixed by running `make clean`. If the error persists, try `make cleanall`. ***
make[1]: *** [/Users/jacobquinn/juliasrc/usr/lib/julia/sys-o.a] Error 1
make: *** [julia-sysimg-release] Error 2

I also added the other check because I ran into:

Generating precompile statements...┌ Error: Failed to precompile precompile(Tuple{typeof(Base.io_has_tvar_name), Base.IOContext{REPL.Terminals.TTYTerminal}, Symbol, Type{Vararg{Any, N} where N}})
└ @ Main.anonymous /Users/jacobquinn/juliasrc/contrib/generate_precompile.jl:171
ERROR: LoadError: LoadError: TypeError: in Type, in parameter, expected Type, got Vararg{Any,N} where N
Stacktrace:
 [1] top-level scope at string:1
 [2] include_string(::Module, ::String, ::String) at ./loading.jl:1064
 [3] include_string at ./loading.jl:1068 [inlined]
 [4] macro expansion at /Users/jacobquinn/juliasrc/contrib/generate_precompile.jl:169 [inlined]
 [5] macro expansion at ./util.jl:212 [inlined]
 [6] (::getfield(Main.anonymous, Symbol("##2#6")){Float64,Base.DevNull})(::String, ::IOStream) at /Users/jacobquinn/juliasrc/contrib/generate_precompile.jl:163
 [7] mktemp(::getfield(Main.anonymous, Symbol("##2#6")){Float64,Base.DevNull}, ::String) at ./file.jl:577
 [8] mktemp at ./file.jl:575 [inlined]
 [9] generate_precompile_statements() at /Users/jacobquinn/juliasrc/contrib/generate_precompile.jl:70
 [10] top-level scope at /Users/jacobquinn/juliasrc/contrib/generate_precompile.jl:184
in expression starting at string:1
in expression starting at /Users/jacobquinn/juliasrc/contrib/generate_precompile.jl:8
*** This error is usually fixed by running `make clean`. If the error persists, try `make cleanall`. ***
make[1]: *** [/Users/jacobquinn/juliasrc/usr/lib/julia/sys-o.a] Error 1
make: *** [julia-sysimg-release] Error 2

[quinnj]

The first precompile statement errors because Char(0x64000000) throws an error, but it should just be Char('d')

[KristofferC]

#32610 should fix the Char problem.

Blacklisting the whole stdlib seems a bit heavy handed. Can't we just list the methods that fail?

[quinnj]

I mean, the whole stdlib is @printf and @sprintf, which lower to Printf.format, so......no, there aren't any other methods that fail, because there's only one. Can we expedite #32610?

[KristofferC]

Oh, I see. If #32610 fixes the whole thing it would be good to just merge that. Otherwise, I guess we can just blacklist the module but this feels like a method that would be quite nice to precompile so we should try not to forget about it.

[quinnj]

Just a status update here: tests are all passing, so I feel like #32799 and this PR could be merged at any time. Happy to add a NEWS entry if people would like, though there should be no user-facing changes. The one awkwardness in this PR is the skipping of precompiling for the Printf module, since there seems to be some lower/precompile-statement-generation bug, but it sounds like there might be a fix for that.

[quinnj]

Given Stefan's comment, I'll plan on merging this (which includes #32799) in 24 hours, unless anyone else wants me to hold off for review or other concerns.

[vtjnash]

Looking at this somewhat quickly, I'm a bit more concerned about the implementation of this and leaves me wondering what we should do before the freeze. I'd be somewhat interested to know if the compile-time dispatch is essential, because that seems to be adding a lot of complexity here and performance cliffs here. It also looked liked you had to hard-code all of the type behaviors so that it's not really extensible anymore.

Finally, is anyone else currently extending Printf.jl that'll be broken by this? While I get that it was hard to extend the old code, it was possible, so someone might be depending on that, but I just don't know. We could move the existing Printf (and Grisu) code inside the printf module and move it to Compat to preserve that API as a down-grade version option as a preparatory step.

[jmkuhn]

DecFP.jl extends Base.Printf.fix_dec and Base.Printf.ini_dec to allow printf formatting. I haven't looked at the details of this PR closely enough to determine how printf formatting could be implemented for DecFP after this PR.

[quinnj]

Yeah, it looks like ArbNumerics.jl, QuadMath.jl, DoubleFloats.jl, and DecFP.jl are the only packages that extended Printf. I'll look into their usages to see what we can do for compat here.

[quinnj]

Quick update on Printf package extenders:

• ArbNumerics.jl just converts to BigFloat, so that's easy
• QuadMath.jl calls it's own internal quadmath_snprintf function, which mirrors how we handle BigFloats, so also not very hard to cover
• DoubleFloats.jl just convert to their respective Float cousin (i.e. Double64 => Float64), so also easy
• DecFP.jl is more tricky, since they're directly implementing the Printf.ini_ routines. I'm not sure what the best thing to do here though. Maybe we can include a little helper that takes the ini_ outputs and does the formatting for you.

@vtjnash, I looked into removing the conversion specifier dispatching, but the result wasn't very satisfying. You end up with formatints, formatstrings, formatfloats, etc. functions that basically mirror the dispatched versions. The performance hit on not dispatching was about 20-30% slower, but for some reason even slower for floats (I didn't quite get to the bottom of why). Also, the partial loop unrolling that I assume you're referring to w/ the "performance cliffs" remark is orthogonal to the conversion specifier type parameter. The loop unrolling helps because your signature is essentially printf(fmt, args...), and with heterogenously typed args, just looping over them causes dynamic dispatch. In the case I've included, we partially unroll up to 8 args, which I would assume probably covers 99% of cases, and it makes a huge difference performance-wise to have each arg type-stably formatted.

So all-in-all, I'm inclined to keep the dispatch mechanics since it's a nice performance boost, while the "world" of possible types is still pretty small.

[quinnj]

Ok, I've cleaned things up a bit more and added a new Printf.tofloat function. This would allow things like:

# for DoubleFloats.jl
Printf.tofloat(x::Double64) = Float64(x)
Printf.tofloat(x::Double32) = Float32(x)
Printf.tofloat(x::Double16) = Float16(x)

# for ArbNumerics.jl
Printf.tofloat(x::ArbFloat) = BigFloat(x)

For QuadMath.jl, I modified the string(f::Spec; modifier="") function so you can pass in a custom modifier (like we do for BigFloats), so for QuadMath, it could simply define:

function Printf.fmt(buf, pos, arg::Float128, spec::Printf.Spec{T}) where {T <: Printf.Floats}
    ptr = pointer(buf, pos)
    newpos = ccall((:quadmath_snprintf, : libquadmath), Cint, (Ptr{UInt8}, Csize_t, Ptr{UInt8}, Cfloat128), ptr, length(buf), string(spec; modifier="Q"), d)
    return pos + newpos
end

So those both seem like good solutions. I'll keep noodling on the DecFP.jl case.

[quinnj]

The 32-bit windows build failed w/ something about reinterpretarray.jl or higherorderfns.jl crashes? Have we seen anything like that before? The freebsd build failed on something related to libgit2. It doesn't seem like either of those issues are related to changes here, but speak up if you think otherwise.

[quinnj]

Ok, I slept on the DecFP.jl situation and I think I have a plan that could work well, it looks something like:

• Include the fix_dec and ini_dec functions in Printf for package extension; this allows DecFP not to break w/ these updates
• Provide a Printf.digits(x, hex) method in Printf for more general extension; the API is you overload Printf.digits(x::MyNumericType, hex::Bool) and return a buffer w/ digit chars for your numeric type, along w/ where the decimal point should be printed (pretty much a combined version of fix_dec, ini_dec, fix_hex, and ini_hex); the fallback definition would call ini_dec, fix_dec, etc.
• Provide a fallback fmt function that would first call digits(x, hex), and then apply the various spacing/padding/flag operations on the returned digits.

We already have most of this logic lying around, so I don't think it will be too much lift to add it in and get it working.

[quinnj]

Ok, I pushed some compat code in the form of fmtfallback that calls the ini_dec and fix_dec code that DecFP.jl calls. I've confirmed that DecFP tests all pass w/ this code; so we now have a compat plan in place for every known extender of Printf.

[vtjnash]

Also, the partial loop unrolling that I assume you're referring to w/ the "performance cliffs" remark is orthogonal to the conversion specifier type parameter

That, and also the dispatch penalty of putting all of this into type-parameters computed from runtime values. Just doesn't seem like a great trade-off.

[quinnj]

@StefanKarpinski, do you still want to review this before merging? Looks like it needs a rebase, but I'm happy to merge if you don't think you'll be able to get to it any time soon.

[DilumAluthge]

I think this PR might have broken ProgressMeter: timholy/ProgressMeter.jl#167

It looks like the offending line in the ProgressMeter source code might be this line.

Is this something that needs to be fixed in Julia or in the ProgressMeter package?

[martinholters]

Seems to be a bug in the new printf code:

julia> @sprintf "%%%s" "a" # ok
"%%a"

julia> @sprintf "%s%%%s" "a" # should require two args
"a%%%s"

julia> @sprintf "%s%%%s" "a" "b" # should produce "a%%b"
ERROR: ArgumentError: mismatch between # of format specifiers and provided args: 1 != 2

[quinnj]

Ok, fix for problem w/ escaped '%' is up: #37470.