Make tab extension lazy #684
Conversation
|
Makes sense to me, thanks! Just kicked of CI to verify. |
| @@ -346,21 +347,20 @@ pub(crate) enum TabExpandedString { | |||
| NoTabs(Cow<'static, str>), | |||
| 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
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.