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

Upgrade blueprint benchmark template #17

Closed
wants to merge 1 commit into from

Conversation

stephannv
Copy link

I refactored some code and due to #16 I noticed that HTML.escape slow down the rendering, so I made available a version to those who controls the content being rendered and want to focus on performance. And some refactoring improved the safe blueprint too, from ~9x to ~5x, I think it was the reuse of the same buffer to build element attributes instead creating a new one for each element.

          ecr   3.45M (289.48ns) (± 1.51%)  4.27kB/op        fastest
      to_html   1.88M (533.03ns) (± 1.62%)  5.52kB/op   1.84× slower
raw blueprint   1.07M (938.61ns) (± 0.90%)   4.9kB/op   3.24× slower
    blueprint 628.81k (  1.59µs) (± 1.27%)   4.9kB/op   5.49× slower
 html_builder 117.60k (  8.50µs) (± 2.56%)  10.4kB/op  29.37× slower
        water 114.54k (  8.73µs) (± 0.68%)  11.2kB/op  30.16× slower
      markout  89.33k ( 11.19µs) (± 0.96%)  15.6kB/op  38.67× slower

@sbsoftware
Copy link
Owner

Hey @stephannv, I'd rather avoid having multiple variants of the same shard in this benchmark. I think it would be enough to include the raw version only - so far, to_html.cr does no escaping either, so comparing it to the variant of blueprint featuring it seems unfair anyway. Maybe we can add a second benchmark at some point which will include more features. What do you think?

@stephannv
Copy link
Author

I think it's fair. But if you want to keep only one, I prefer the Blueprint::HTML, but it's up to you to decide.

I opened a PR to html_builder, improving its performance, it went from ~29x to ~5x.

                           ecr   3.42M (292.20ns) (± 1.63%)  4.27kB/op        fastest
            html_builder (New) 664.95k (  1.50µs) (± 2.18%)  8.39kB/op   5.15× slower
        html_builder (Current) 118.40k (  8.45µs) (± 0.74%)  10.4kB/op  28.90× slower

So I think it makes sense to close this PR and let you upgrade the dependencies when you think it best to do so, so you can upgrade multiple libs and update the benchmark result.

@stephannv stephannv closed this Sep 11, 2024
@sbsoftware
Copy link
Owner

I'll keep the one you prefer for now then. At some point in the future I might very well implement some form of escaping for to_html.cr, too, and then I will decide how to handle the different cases in the benchmark.

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

Successfully merging this pull request may close these issues.

2 participants