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

perf: short-circuit lean_string_append #642

Closed
wants to merge 3 commits into from

Conversation

Kha
Copy link
Member

@Kha Kha commented Aug 21, 2021

No need to allocate/copy when either side is the empty string

@Kha
Copy link
Member Author

Kha commented Aug 21, 2021

!bench

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit 5704009.

  Benchmark   Metric    Change
  =====================================
- stdlib      parsing       2% (72.9 σ)

@Kha
Copy link
Member Author

Kha commented Aug 21, 2021

Huh, interesting

@Kha
Copy link
Member Author

Kha commented Aug 21, 2021

!bench

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit ce3b065.

  Benchmark   Metric    Change
  ======================================
- stdlib      parsing       2% (368.8 σ)

@Kha
Copy link
Member Author

Kha commented Aug 21, 2021

Oh well.

@tydeu
Copy link
Member

tydeu commented Aug 22, 2021

Interesting benchmarks! I guess it is at least 8 additional instructions for every copy for an edge case that might not happen that often. Thus, the former may outweigh the later.

@Kha Kha closed this Sep 9, 2021
ChrisHughes24 pushed a commit to ChrisHughes24/lean4 that referenced this pull request Dec 2, 2022
Tracking mathlib commit: [fd47bdf09e90f553519c712378e651975fe8c829](leanprover-community/mathlib@fd47bdf)

Co-authored-by: David Wärn <codwarn@gmail.com>
Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
Co-authored-by: Scott Morrison <scott@tqft.net>
ChrisHughes24 pushed a commit to ChrisHughes24/lean4 that referenced this pull request Dec 2, 2022
- [x] depends on: leanprover#642
- [x] depends on: leanprover-community/mathlib#17730

Tracking mathlib commit: [e50b8c261b0a000b806ec0e1356b41945eda61f7](leanprover-community/mathlib@e50b8c2)

Co-authored-by: David Wärn <codwarn@gmail.com>
Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
Co-authored-by: Scott Morrison <scott@tqft.net>
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

3 participants