HypertextLiteral.jl icon indicating copy to clipboard operation
HypertextLiteral.jl copied to clipboard

Fix regression tests on Julia 1.10

Open clarkevans opened this issue 1 year ago • 4 comments

There are several independent breakages on nightly.

  • #3350 breaks objects that are mutated, Custom to Custom1, Custom2, etc
  • In notation.md, it's just a change in how NamedTuple type is serialized
  • In script.md, looks like hasmethod() no longer works

clarkevans avatar May 23 '23 17:05 clarkevans

This is proably related to your point 3 above, but my code relying on having custom struct interpolation inside

@clarkevans let me know if this is the same problem you mention in point 3 above or if I shall open a new issue

Maybe on 1.10 due to hasmethod becoming inferable at compile time Julia PR 48639 we don't need the @generated function anymore and can just make this a function? EDIT: I tested a non generated function with hasmethod on 1.10 but while it's much faster than in 1.9 it is still around 40ns on my machine as opposed to the ~3ns of the @generated function here and of the ~20ns of a try-catch

disberd avatar Aug 30 '23 07:08 disberd

Probably this is it. I could look at this later. Feel free to commit any updates.

clarkevans avatar Aug 30 '23 15:08 clarkevans

I don't have any clear way forward here unfortunately. Putting checks with hasmethod or try -catch creates a performance penalty compared to the currently generated function or a direct call to show(io, ::MIME"text/javascript", x). I understand the reason why the direct use of show was skipped in favor of the @generated function was to have a clearer error message. Is this correct?

I don't know how much performance critical is this code path, if going from 3ns to 20ns is OK probably the best way to keep the custom error message is to use a try-catch block, although it's not very elegant.

I remember you had extensively optimized the performance of @htl as it was crucial in some of your use case so you might now better whether this has a non-negligible impact

disberd avatar Sep 05 '23 10:09 disberd

@disberd We ended up going with a poorer error message on Julia 1.10+: https://github.com/JuliaPluto/HypertextLiteral.jl/pull/36

fonsp avatar Oct 31 '23 21:10 fonsp