Conversation
|
Makes sense to me, thanks! Just kicked of CI to verify. |
djc
left a comment
djc
left a comment
There was a problem hiding this comment.
Thanks, that looks like a nice win!
Also, would be nice if you can squash your commits.
| WithTabs { | ||
| original: Cow<'static, str>, | ||
| expanded: String, | ||
| expanded: OnceCell<String>, |
There was a problem hiding this comment.
Can we just use Option instead of OnceCell for this? Seems simpler.
There was a problem hiding this comment.
No, I don't think so: We don't have mut access in expanded. AFAIK, that's essentially what OnceCell is there for.
jaheba
force-pushed
the
lazy-tab-expansion
branch
from
e8de6a3 to
803b013
Compare
|
This change seems to have made |
|
@tgross35 ah, sorry about that. Would you be able to submit a PR, including a test for |
The change at [1] stores some data in a `OnceCell`, which made `ProgressStyle` no longer `Sync`. Fix this by changing the `OnceCell` to a `OnceLock`, and add a static test that all relevant public types are `Send` and `Sync`. [1]: console-rs#684
|
Sure! Done at #694 |
The change at [1] stores some data in a `OnceCell`, which made `ProgressStyle` no longer `Sync`. Fix this by changing the `OnceCell` to a `OnceLock`, and add a static test that all relevant public types are `Send` and `Sync`. [1]: #684
4 participants
Running the snippet of #683 a big chunk of the time is spent in allocating a new string in
TabExpandedString. Delaying the expanding of tabs reduces the runtime of ~100ms to ~60ms.The idea is simple: Rather than replacing
\twith spaces immediately, this is done onceexpandedis actually called.