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

Avoid dyn in to_html #54

Closed
wants to merge 1 commit into from
Closed

Avoid dyn in to_html #54

wants to merge 1 commit into from

Conversation

kornelski
Copy link
Contributor

Makes benchmarks slightly better:

 name                before ns/iter  after ns/iter  diff ns/iter   diff %  speedup
 escaped_long        75,357          70,066               -5,291   -7.02%   x 1.08
 escaped_no_op       92,623          74,609              -18,014  -19.45%   x 1.24
 escaped_nums        104,926         81,901              -23,025  -21.94%   x 1.28
 escaped_short_dyn   123,600         105,811             -17,789  -14.39%   x 1.17
 escaped_short_impl  113,133         90,689              -22,444  -19.84%   x 1.25
 raw                 27,895          26,434               -1,461   -5.24%   x 1.06

(the comparison is with improved benchmark code in both cases)

@kornelski
Copy link
Contributor Author

Blocked by #57

@kaj
Copy link
Owner

kaj commented Nov 1, 2019

Hi! Could you rebase now that #57 is fixed and #55 is merged? I'm sorry for the long delay in the later.

@kaj
Copy link
Owner

kaj commented Dec 10, 2020

Hm. One probem with this is that it makes it impossible to handle a dyn ToHtml, which I use in some examples and think is a good thing in some real cases. Is the performance improvement worth that?

On the other hand, this would improve consistency between a template and a ToHtm impl, making them take the same kind of output argument, which would probably make #88 easier to fix.

@kornelski
Copy link
Contributor Author

You're right. Object safety is a feature.

@kornelski kornelski closed this Dec 10, 2020
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.

None yet

2 participants