Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Purity of low level functions / performance #107

Open
t-bltg opened this issue Jan 19, 2025 · 1 comment
Open

Purity of low level functions / performance #107

t-bltg opened this issue Jan 19, 2025 · 1 comment

Comments

@t-bltg
Copy link

t-bltg commented Jan 19, 2025

Some low level StyledStrings functions are not pure, they depend on global states e.g. Base.have_truecolor, ...

E.g.:

StyledStrings.jl/src/io.jl

Lines 116 to 120 in 8985a37

if Base.get_have_truecolor()
termcolor24bit(io, color.value, category)
else
termcolor8bit(io, color.value, category)
end

which induces annoying workarounds such as
prev_terminfo = getglobal(Base, :current_terminfo)
prev_truecolor = getglobal(Base, :have_truecolor)
try
setglobal!(Base, :current_terminfo, tinfo)
setglobal!(Base, :have_truecolor, haskey(tinfo, :setrgbf))
fn()
finally
setglobal!(Base, :current_terminfo, prev_terminfo)
setglobal!(Base, :have_truecolor, prev_truecolor)
end

This is currently blocking in my trial PR to transition from Crayons.jl to the StyledsStrings stdlib, for UnicodePlots.jl.

Also, I'm not sure transitioning will lead to a performance statu quo or gain in terms of performance / allocations.

@t-bltg t-bltg changed the title Purity of functions / performance Purity of low level functions / performance Jan 19, 2025
@tecosaur
Copy link
Collaborator

Some low level StyledStrings functions are not pure, they depend on global states e.g. Base.have_truecolor

This is correct. However, you have not articulated the problem with this state of affairs. While pure functions are generally nice to have, IO output is inherently impure, and so I must confess I don't see any automatic issue with an IO-oriented function being impure.

which induces annoying workarounds such as

A bunch of non-standard usage is involved in the test suite to simulate/produce certain conditions that are normally inferred on the running system.

This makes it hard for me to see what sort of issue you're pointing to when you show something unusual/awkward in the test suite to do with system capabilities.

This is currently blocking in my trial PR to transition from Crayons.jl to the StyledsStrings stdlib, for UnicodePlots.jl.

Just taking a glance at the PR, it looks like you've tried to directly switch out Crayons for StyledStrings, and use StyledStrings in the same way as Crayons was in UnicodePlots. I don't anticipate that going particularly well, since Crayons and StyledStrings have rather different approaches to styling.

For instance, UnicodePlots currently handles 4-bit/8-bit/24-bit colour printing itself, but this is one of the problems that StyledStrings is designed to take care of. Trying to take this out of StyledString's hands is fighting against the design of the package, and so is expected to result in ugly code.

Also, I'm not sure transitioning will lead to a performance statu quo or gain in terms of performance / allocations.

When it comes to simply printing out text wrapped with ANSI codes, you can't really beat printstyled, and at the point of printing Crayons is essentially a thin wrapper around printstyled. StyledStrings has a more complex way of representing styled text, in order to allow things like multi-pass styling, and this incurs a performance overhead.

So you can expect StyledStrings to be slower that printstyled/Crayons when it comes to printing content, but I have yet to be aware of any use cases where the performance is an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants