Piotr Osiewicz created
This commit is all about strings, not about line layout at all. When
laying out text, we use a line layout cache to avoid roundtrips to
system layout engine where possible. This makes it so that we might end
up not needing an owned version of text to insert into the cache, as we
might get a cached version.
The API boundary of line layout accepted text to be laid out as &str. It
then performed cache lookup (which didn't require having an owned
version) and only resorted to making an owned version when needed. As it
turned out though, exact cache hits are quite rare and we end up needing
owned version more often than not. The callers of line layout either
dealt with SharedStrings or owned Strings. Due to coercing them into
&str, we were ~always copying text into a new string (unless there was a
same-frame-hit). This is a bit wasteful, thus this PR generifies the API
a bit to make it easier to reuse existing string allocations if there
are any.
Benchmark scenario: scrolling down page-by-page through editor_tests (I
ran the same scenario twice):
Release Notes:
- N/Acrates/gpui/src/shared_string.rs | 6 +++
crates/gpui/src/text_system.rs | 12 ++++--
crates/gpui/src/text_system/line_layout.rs | 43 ++++++++++++++++-------
3 files changed, 44 insertions(+), 17 deletions(-)
@@ -81,6 +81,12 @@ impl<'a> PartialEq<&'a str> for SharedString {
}
}
+impl From<&SharedString> for SharedString {
+ fn from(value: &SharedString) -> Self {
+ value.clone()
+ }
+}
+
impl From<SharedString> for Arc<str> {
fn from(val: SharedString) -> Self {
match val.0 {
@@ -356,7 +356,7 @@ impl WindowTextSystem {
});
}
- let layout = self.layout_line(text.as_ref(), font_size, runs)?;
+ let layout = self.layout_line(&text, font_size, runs)?;
Ok(ShapedLine {
layout,
@@ -483,12 +483,16 @@ impl WindowTextSystem {
/// Subsets of the line can be styled independently with the `runs` parameter.
/// Generally, you should prefer to use `TextLayout::shape_line` instead, which
/// can be painted directly.
- pub fn layout_line(
+ pub fn layout_line<Text>(
&self,
- text: &str,
+ text: Text,
font_size: Pixels,
runs: &[TextRun],
- ) -> Result<Arc<LineLayout>> {
+ ) -> Result<Arc<LineLayout>>
+ where
+ Text: AsRef<str>,
+ SharedString: From<Text>,
+ {
let mut font_runs = self.font_runs_pool.lock().pop().unwrap_or_default();
for run in runs.iter() {
let font_id = self.resolve_font(&run.font);
@@ -1,4 +1,4 @@
-use crate::{point, px, FontId, GlyphId, Pixels, PlatformTextSystem, Point, Size};
+use crate::{point, px, FontId, GlyphId, Pixels, PlatformTextSystem, Point, SharedString, Size};
use collections::FxHashMap;
use parking_lot::{Mutex, RwLock, RwLockUpgradableReadGuard};
use smallvec::SmallVec;
@@ -420,15 +420,19 @@ impl LineLayoutCache {
curr_frame.used_wrapped_lines.clear();
}
- pub fn layout_wrapped_line(
+ pub fn layout_wrapped_line<Text>(
&self,
- text: &str,
+ text: Text,
font_size: Pixels,
runs: &[FontRun],
wrap_width: Option<Pixels>,
- ) -> Arc<WrappedLineLayout> {
+ ) -> Arc<WrappedLineLayout>
+ where
+ Text: AsRef<str>,
+ SharedString: From<Text>,
+ {
let key = &CacheKeyRef {
- text,
+ text: text.as_ref(),
font_size,
runs,
wrap_width,
@@ -449,8 +453,8 @@ impl LineLayoutCache {
layout
} else {
drop(current_frame);
-
- let unwrapped_layout = self.layout_line(text, font_size, runs);
+ let text = SharedString::from(text);
+ let unwrapped_layout = self.layout_line::<&SharedString>(&text, font_size, runs);
let wrap_boundaries = if let Some(wrap_width) = wrap_width {
unwrapped_layout.compute_wrap_boundaries(text.as_ref(), wrap_width)
} else {
@@ -462,7 +466,7 @@ impl LineLayoutCache {
wrap_width,
});
let key = Arc::new(CacheKey {
- text: text.into(),
+ text,
font_size,
runs: SmallVec::from(runs),
wrap_width,
@@ -478,9 +482,18 @@ impl LineLayoutCache {
}
}
- pub fn layout_line(&self, text: &str, font_size: Pixels, runs: &[FontRun]) -> Arc<LineLayout> {
+ pub fn layout_line<Text>(
+ &self,
+ text: Text,
+ font_size: Pixels,
+ runs: &[FontRun],
+ ) -> Arc<LineLayout>
+ where
+ Text: AsRef<str>,
+ SharedString: From<Text>,
+ {
let key = &CacheKeyRef {
- text,
+ text: text.as_ref(),
font_size,
runs,
wrap_width: None,
@@ -497,9 +510,13 @@ impl LineLayoutCache {
current_frame.used_lines.push(key);
layout
} else {
- let layout = Arc::new(self.platform_text_system.layout_line(text, font_size, runs));
+ let text = SharedString::from(text);
+ let layout = Arc::new(
+ self.platform_text_system
+ .layout_line(&text, font_size, runs),
+ );
let key = Arc::new(CacheKey {
- text: text.into(),
+ text,
font_size,
runs: SmallVec::from(runs),
wrap_width: None,
@@ -524,7 +541,7 @@ trait AsCacheKeyRef {
#[derive(Clone, Debug, Eq)]
struct CacheKey {
- text: String,
+ text: SharedString,
font_size: Pixels,
runs: SmallVec<[FontRun; 1]>,
wrap_width: Option<Pixels>,