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

replace askama_escape in favor of v_htmlescape #2824

Merged
merged 3 commits into from Aug 30, 2022
Merged

replace askama_escape in favor of v_htmlescape #2824

merged 3 commits into from Aug 30, 2022

Conversation

zzau13
Copy link
Contributor

@zzau13 zzau13 commented Jul 26, 2022

Feature

n the new version, v_htmlescape@0.15, it has no dependencies and is a generated file of about 1500 lines of code. Compile time is minimal and it is optimized with simd instructions on x86 and x86_64.

Sorry for taking so long.

PR Checklist

  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • A changelog entry has been made for the appropriate packages.
  • Format code with the latest stable rustfmt.
  • (Team) Label with affected crates and semver status.

Previous PR: #2402

@robjtede robjtede added A-files project: actix-files B-semver-patch labels Jul 26, 2022
@robjtede robjtede requested a review from a team July 26, 2022 23:35
@cetra3
Copy link
Contributor

cetra3 commented Jul 30, 2022

Lots of unsafe in this lib, while there is none I could see in askama.

Is there any benefit here besides less dependencies? Is it faster to use?

@robjtede
Copy link
Member

I'm assuming the unsafe is largely to do with running SSE/AVX code.

@zzau13
Copy link
Contributor Author

zzau13 commented Jul 30, 2022

If necessary I can put more test coverage. And I have had the fuzzy tests for hours without failures. Anything you need, say it and I'll implement it.

@zzau13
Copy link
Contributor Author

zzau13 commented Aug 8, 2022

@cetra3 https://github.com/rust-lang/rust/search?q=unsafe

Is there a problem that needs to be fixed? This library was already used before in actix for a long time and has not given any problem. And of course it is many times faster than anyone. I invite you to look at the benchmarks.

@robjtede robjtede merged commit c993055 into actix:master Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-files project: actix-files B-semver-patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants