Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Improved speedups#519

Open
thewhaleking wants to merge 5 commits into
pallets:main from
thewhaleking:main
Open

Improved speedups #519
thewhaleking wants to merge 5 commits into
pallets:main from
thewhaleking:main

Conversation

@thewhaleking

@thewhaleking thewhaleking commented Mar 10, 2026
edited
Loading

Copy link
Copy Markdown

Came across this older PR: #438 which had no activity in quite some time. Thought the improved speed is a good idea, but I don't really know Rust, so I implemented the changes in C.

The benchmarks seem good:

 ┌──────────────┬─────────┬────────────────┬────────────────┬────────────────┐
 │ Benchmark │ Native │ Old C speedups │ New C speedups │ Rust (from PR) │
 ├──────────────┼─────────┼────────────────┼────────────────┼────────────────┤
 │ short escape │ 438 ns │ ~417 ns │ 227 ns │ 522 ns │
 ├──────────────┼─────────┼────────────────┼────────────────┼────────────────┤
 │ long escape │ 23.5 μs │ ~7.79 μs │ 2.60 μs │ 6.71 μs │
 ├──────────────┼─────────┼────────────────┼────────────────┼────────────────┤
 │ short plain │ 333 ns │ ~349 ns │ 182 ns │ 401 ns │
 ├──────────────┼─────────┼────────────────┼────────────────┼────────────────┤
 │ long plain │ 23.4 μs │ ~7.77 μs │ 2.60 μs │ 6.73 μs │
 ├──────────────┼─────────┼────────────────┼────────────────┼────────────────┤
 │ long suffix │ 183 μs │ ~131 μs │ 48.4 μs │ 58.3 μs │
 └──────────────┴─────────┴────────────────┴────────────────┴────────────────┘

Running on 2024 MacBook Pro M4 Pro
The Rust column is from the PR, as I wasn't actually able to get it to run locally.

I didn't add tests, because it seemed like this behavior is already covered.

davidism commented Mar 10, 2026
edited
Loading

Copy link
Copy Markdown
Member

Thanks, cool to see that it works well in C too. I still plan to use the Rust version, as I'm also interested in the easier maintainability and safety of it. The speed isn't too important to me at this point, since it's already so fast, we were benchmarking there to make sure it wasn't unreasonably slow. If you're able, try running the rust yourself so the timings are comparable, that would be interesting to see.

Copy link
Copy Markdown
Author

Thanks, cool to see that it works well in C too. I still plan to use the Rust version, as I'm also interested in the easier maintainability and safety of it. The speed isn't too important to me at this point, since it's already so fast, we were benchmarking there to make sure it wasn't unreasonably slow. If you're able, try running the rust yourself so the timings are comparable, that would be interesting to see.

That's understandable. I think once Rust matures a bit (3-5 years) it will be really useful in the Python ecosystem. I think the current instability of the Rust-Python workflows (with like PyO3) put it in a maintenance nightmare at this point.

I wasn't able to run the Rust tests myself. I have cargo installed, and build tools with it frequently, but pip install . did not work for me, and I didn't really want to spend time debugging it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /