opt-level and max-methods = 1

[chriselrod]

Trying to reduce latency.
After:

Test Summary: | Pass  Total  Time
macro test    |   25     25  2.6s
Test Summary: | Pass  Total  Time
recipe test   |   13     13  4.4s
Test Summary:  | Pass  Total  Time
latexify tests |   10     10  1.3s
Test Summary:  | Pass  Broken  Total  Time
latexraw tests |  133       1    134  4.8s
Test Summary:    | Pass  Total  Time
latexalign tests |    4      4  1.3s
Test Summary:    | Pass  Total  Time
latexarray tests |   15     15  1.5s
Test Summary:       | Pass  Total  Time
latexequation tests |    2      2  0.1s
Test Summary:      | Pass  Total  Time
latexbracket tests |    2      2  0.0s
Test Summary:     | Pass  Total  Time
latexinline tests |    1      1  0.3s
Test Summary:      | Pass  Total  Time
latextabular tests |   11     11  7.3s
WARNING: using DataFrames.head in module Main conflicts with an existing identifier.
Test Summary: | Pass  Total  Time
mdtable tests |   16     16  2.7s
Test Summary:    | Pass  Broken  Total  Time
DataFrame Plugin |    3       1      4  0.6s
Test Summary:    | Pass  Total  Time
SymEngine Plugin |    1      1  0.5s

Before:

Test Summary: | Pass  Total  Time
macro test    |   25     25  3.5s
Test Summary: | Pass  Total  Time
recipe test   |   13     13  5.4s
Test Summary:  | Pass  Total  Time
latexify tests |   10     10  1.6s
Test Summary:  | Pass  Broken  Total  Time
latexraw tests |  133       1    134  5.9s
Test Summary:    | Pass  Total  Time
latexalign tests |    4      4  2.0s
Test Summary:    | Pass  Total  Time
latexarray tests |   15     15  2.1s
Test Summary:       | Pass  Total  Time
latexequation tests |    2      2  0.1s
Test Summary:      | Pass  Total  Time
latexbracket tests |    2      2  0.1s
Test Summary:     | Pass  Total  Time
latexinline tests |    1      1  0.3s
Test Summary:      | Pass  Total  Time
latextabular tests |   11     11  7.8s
WARNING: using DataFrames.head in module Main conflicts with an existing identifier.
Test Summary: | Pass  Total  Time
mdtable tests |   16     16  3.1s
Test Summary:    | Pass  Broken  Total  Time
DataFrame Plugin |    3       1      4  0.6s
Test Summary:    | Pass  Total  Time
SymEngine Plugin |    1      1  0.5s

I was hoping for more.

[codecov]

Codecov Report

Base: 72.23% // Head: 72.23% // No change to project coverage 👍

Coverage data is based on head (22d1905) compared to base (ddf49bd).
Patch has no changes to coverable lines.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #244   +/-   ##
=======================================
  Coverage   72.23%   72.23%           
=======================================
  Files          23       23           
  Lines         940      940           
=======================================
  Hits          679      679           
  Misses        261      261           

Impacted Files	Coverage Δ
src/Latexify.jl	46.15% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[chriselrod]

Some performance benchmarks on a big file of latexify tests:
myb/concrete_only, latexify 0.15.13:

julia> using Latexify, Pumas, Test, StaticArrays
WARNING: using StaticArrays.pop in module Main conflicts with an existing identifier.

julia> @time include("latexify_recipes.jl")
326.492189 seconds (335.92 M allocations: 15.622 GiB, 42.37% gc time, 57.01% compilation time: <1% of which was recompilation)
Test Passed

Julia master, latexify 0.15.13:

julia> @time include("latexify_recipes.jl")
1104.623542 seconds (1.15 G allocations: 78.342 GiB, 29.51% gc time, 88.57% compilation time: 2% of which was recompilation)
Test Passed

Note that this Julia branch has no negative performance impact on fast, type stable, code.
It needs more testing, but ODE solves, @turbo loops, etc are just as fast as on Julia master, while slow code that spends an eternity compiling (like Latexify.jl) is far faster.

I was hoping the tweaks in this PR would also give a 2 or 3x improvement, but alas, we'll have to wait for some form of this branch to make it into Julia, or improve this library.

[chriselrod]

myb/concrete_only with this PR:

julia> @time include("latexify_recipes.jl")
327.410396 seconds (337.46 M allocations: 15.694 GiB, 42.77% gc time, 56.58% compilation time: <1% of which was recompilation)

Master with this PR:

julia> @time include("latexify_recipes.jl")
WARNING: using StaticArrays.pop in module Main conflicts with an existing identifier.
1519.625729 seconds (1.20 G allocations: 82.093 GiB, 24.20% gc time, 91.87% compilation time: 1% of which was recompilation)

So, this PR seems to have regressed a lot.

[chriselrod]

Latest Latexify release:
Julia myb/concrete_only:

julia> @time include("latexify_recipes.jl")
330.552732 seconds (337.50 M allocations: 15.696 GiB, 42.14% gc time, 58.56% compilation time: <1% of which was recompilation)
Test Passed

Julia master:

julia> @time include("latexify_recipes.jl")
1124.050085 seconds (1.15 G allocations: 78.384 GiB, 31.80% gc time, 87.93% compilation time: 2% of which was recompilation)
Test Passed

[gustaphe]

Is there somewhere we can read about this? Is the myb branch "official" in that these commands are guaranteed to not mean something else in an upcoming Julia release? Or is there maybe a pull request this should wait for?

latexify should not be performance critical, but lower latency is always nice :)

[ChrisRackauckas]

These commands have been around for years. It's what helped Plots.jl improve its load time in like 2020. opt-level=1 is just the same as forcing -O1 on the module, and max-methods=1 is just disabling union splitting optimizations. Just search and you'll find tons about this because they are not new options, just Yingbo's branch improves a few things with them.

[korsbo]

@gustaphe runtime is usually not a problem but there are some real latency issues. The worst ones I've seen is about 30 seconds where there are recipes calling other recipes. That absolutely precludes latexify from being used in the show methods in those cases so there are cases where latency is a real problem.

[gustaphe]

Okay, as far as I knew this was related to the myb branch. A cursory search mostly returns results about "max vs maximum" and Optim.jl respectively.

If it's that stable and gives these speedups, LGTM.

[ChrisRackauckas]

This package is missing snoopprecompile: that seems to be a major oversight given its mostly a compile time burdened library?

[korsbo]

I have tried these macros myself, probably back in julia 1.6, but with no tangible effect so I never pushed them. I was hoping that my rewrite (which has been stalled for like 2 years) would fix the issue but I'm not entirely sure that it does either. That rewrite would incur a massive amount of downstream work since it's breaking the current recipe system, hence my tardiness of getting it done.

[korsbo]

Yeah, SnoopPreCompile would certainly be worth a shot. I suspect that much of the bad performance is coming from the recipes which I don't think would be fixed by this. But it could still make an impact and it's worth adding.

[chriselrod]

No, I was noting that the myb/concrete_only Julia branch tweaked inference heuristics, leading to a 3x+ improvement for Latexify.

However, tweaking the options we have available in an official release (which this PR does) didn't really seem to help much at all.

[chriselrod]

This package is missing snoopprecompile: that seems to be a major oversight given its mostly a compile time burdened library?

I don't think so, because this package seems to always compile for every new thing it has to do.
It seems like it is severely over specializing.
But I did try adding a @nospecialize, which didn't seem to help much either, so perhaps I am wrong there/should try it more.

I'm speaking in complete ignorance about the internals of this library.
Just that I don't think that test script I ran should take anywhere close to 20 minutes. 5.5 minutes (which myb/concrete_only achieves) is better, but still way too long IMO.

[ChrisRackauckas]

I don't think so, because this package seems to always compile for every new thing it has to do.
It seems like it is severely over specializing.

Oh that's not good 😅 . I would assume that something based around printing like this would just use one set of unparameterized types over symbols, similar to writing a compiler. If that's not the case, I guess the good thing is that there is big room for improvement.

[chriselrod]

I don't think so, because this package seems to always compile for every new thing it has to do.
It seems like it is severely over specializing.

Oh that's not good sweat_smile . I would assume that something based around printing like this would just use one set of unparameterized types over symbols, similar to writing a compiler. If that's not the case, I guess the good thing is that there is big room for improvement.

@korsbo has been working on a rewrite that hopefully improves this, but it breaks the recipe system, and updating all downstream packages would be an ordeal...

[chriselrod]

I think max_methods=1 is generally safe and should be the defualt in Julia (LoopVectorization.jl even uses it -- not because it makes a difference, but to emphasize that I think it is mostly harmless).

That's not the case for setting the opt-level to 1, though.

So, I think these are all reasonable options with respect to this PR:

• Merge as is
• Drop the opt level change, merge only max_methods
• Close

[korsbo]

Yeah, let's merge this. It might not do all the magic one would have hoped for but latency is painful while runtime is insignificant so anything that tries to balance the two more favourably is a good step.