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

Look in to using bisect module to replace home grown binary searches. #3298

Open
willmcgugan opened this issue Mar 4, 2024 · 3 comments · May be fixed by #3300
Open

Look in to using bisect module to replace home grown binary searches. #3298

willmcgugan opened this issue Mar 4, 2024 · 3 comments · May be fixed by #3300
Assignees

Comments

@willmcgugan
Copy link
Collaborator

willmcgugan commented Mar 4, 2024

I implemented a few binary searches in Rich, such as getting the cell length of a character. I wonder if we can replace those binary searches with the bisect module. If that is implemented in C, which I imagine it is, then it could be a speed win.

Suggest we try it, and if it works profile the improvement (if any).

Copy link

github-actions bot commented Mar 4, 2024

Thank you for your issue. Give us a little time to review it.

PS. You might want to check the FAQ if you haven't done so already.

This is an automated reply, generated by FAQtory

@rodrigogiraoserrao
Copy link
Contributor

These are the binary searches I found:

  • rich/cells.py::_get_codepoint_cell_size
  • rich/cells.py::set_cell_size
  • rich/text.py::Text.divide
  • tools/make_terminal_widths.py (irrelevant)

@rodrigogiraoserrao rodrigogiraoserrao self-assigned this Mar 4, 2024
@rodrigogiraoserrao
Copy link
Contributor

Segment.divide and Segment._split_cells use a linear search that could potentially benefit from binary search.

@rodrigogiraoserrao rodrigogiraoserrao linked a pull request Mar 7, 2024 that will close this issue
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 a pull request may close this issue.

2 participants