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

ft_printf can be very slow #7

Closed
RalphAS opened this issue Feb 2, 2019 · 5 comments
Closed

ft_printf can be very slow #7

RalphAS opened this issue Feb 2, 2019 · 5 comments

Comments

@RalphAS
Copy link

RalphAS commented Feb 2, 2019

Although the slowness is acknowledged in the documentation, it is extreme for some format strings. I have found this pattern to be a good alternative:

using Formatting
ff = generate_formatter("%.3g")
ft_ff = Dict{Int,Function}(0 => (v,i) -> ff(v))

pretty_table(table, header, formatter=ft_ff)

If dependence on Formatting.jl is undesirable, it would still be helpful to mention this in the documentation. (The above format takes several seconds for 100 numbers with wide dynamic range using ft_printf, vs several millisec with Formatting.)

@ronisbr
Copy link
Owner

ronisbr commented Feb 2, 2019

Thanks for the tip @RalphAS .

What if we create another formatter called ft_formatting (or something) in which the input is a string that can be used in generate_formatter? Can you test how bad is the performance if genenerate_formatter is called inside the formatter function?

Btw, I really don't mind to have Formating.jl as a dependence since it loads very quickly, actually it would be pretty good if this can improve the performance :)

@ronisbr
Copy link
Owner

ronisbr commented Feb 2, 2019

Wow! I just did a test here and I got 25s from ft_printf and 0.06s from ft_formatting. I will commit something now to add this feature. To make the nomenclature more close to Formatting.jl, I will name this predefined formatter as ft_fmtstr.

@ronisbr
Copy link
Owner

ronisbr commented Feb 2, 2019

Actually, I am having problem with generate_formatter. I think it is because the way it was constructer will not be possible to do this as I thought. I keep getting errors like:

ERROR: MethodError: no method matching (::getfield(Formatting, Symbol("##5#6")))(::Float64)
The applicable method may be too new: running in world age 25580, while current world is 25581.

However, I think it is way better to just replace the @sprintf macro to the function sprintf1. The speed gain is amazing!

@ronisbr ronisbr closed this as completed in 4c530fe Feb 2, 2019
@ronisbr
Copy link
Owner

ronisbr commented Feb 2, 2019

Done, thanks @RalphAS for the tip. The performance is much, much better now. Is it possible to test before I tag the new version? You just need to use the same ft_printf with the same parameters. The performance should be close to your example now.

@RalphAS
Copy link
Author

RalphAS commented Feb 3, 2019

Your new version works well for my typical cases; thanks. You might need to cache or memoize functions to use generate_formatter, so it's probably not worth the trouble.

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