fix: Optimize dynamic string building (#491)

Author: mwcampbell

consumer/src/node.rs

@@ -17,7 +17,7 @@ use alloc::{
     sync::Arc,
     vec::Vec,
 };
-use core::iter::FusedIterator;
+use core::{fmt, iter::FusedIterator};
 
 use crate::filters::FilterResult;
 use crate::iterators::{
@@ -501,20 +501,37 @@ impl<'a> Node<'a> {
     }
 
     pub fn label(&self) -> Option<String> {
+        let mut result = String::new();
+        self.write_label(&mut result).unwrap().then_some(result)
+    }
+
+    fn write_label_direct<W: fmt::Write>(&self, mut writer: W) -> Result<bool, fmt::Error> {
         if let Some(label) = &self.data().label() {
-            Some(label.to_string())
+            writer.write_str(label)?;
+            Ok(true)
+        } else {
+            Ok(false)
+        }
+    }
+
+    pub fn write_label<W: fmt::Write>(&self, mut writer: W) -> Result<bool, fmt::Error> {
+        if self.write_label_direct(&mut writer)? {
+            Ok(true)
         } else {
-            let labels = self
-                .labelled_by()
-                .filter_map(|node| {
-                    if node.label_comes_from_value() {
-                        node.value()
-                    } else {
-                        node.label()
-                    }
-                })
-                .collect::<Vec<String>>();
-            (!labels.is_empty()).then(move || labels.join(" "))
+            let mut wrote_one = false;
+            for node in self.labelled_by() {
+                let writer = SpacePrefixingWriter {
+                    inner: &mut writer,
+                    need_prefix: wrote_one,
+                };
+                let wrote_this_time = if node.label_comes_from_value() {
+                    node.write_value(writer)
+                } else {
+                    node.write_label_direct(writer)
+                }?;
+                wrote_one = wrote_one || wrote_this_time;
+            }
+            Ok(wrote_one)
         }
     }
 
@@ -529,12 +546,19 @@ impl<'a> Node<'a> {
     }
 
     pub fn value(&self) -> Option<String> {
+        let mut result = String::new();
+        self.write_value(&mut result).unwrap().then_some(result)
+    }
+
+    pub fn write_value<W: fmt::Write>(&self, mut writer: W) -> Result<bool, fmt::Error> {
         if let Some(value) = &self.data().value() {
-            Some(value.to_string())
+            writer.write_str(value)?;
+            Ok(true)
         } else if self.supports_text_ranges() && !self.is_multiline() {
-            Some(self.document_range().text())
+            self.document_range().write_text(writer)?;
+            Ok(true)
         } else {
-            None
+            Ok(false)
         }
     }
 
@@ -664,6 +688,33 @@ impl<'a> Node<'a> {
     }
 }
 
+struct SpacePrefixingWriter<W: fmt::Write> {
+    inner: W,
+    need_prefix: bool,
+}
+
+impl<W: fmt::Write> SpacePrefixingWriter<W> {
+    fn write_prefix_if_needed(&mut self) -> fmt::Result {
+        if self.need_prefix {
+            self.inner.write_char(' ')?;
+            self.need_prefix = false;
+        }
+        Ok(())
+    }
+}
+
+impl<W: fmt::Write> fmt::Write for SpacePrefixingWriter<W> {
+    fn write_str(&mut self, s: &str) -> fmt::Result {
+        self.write_prefix_if_needed()?;
+        self.inner.write_str(s)
+    }
+
+    fn write_char(&mut self, c: char) -> fmt::Result {
+        self.write_prefix_if_needed()?;
+        self.inner.write_char(c)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use accesskit::{Node, NodeId, Point, Rect, Role, Tree, TreeUpdate};

consumer/src/text.rs

@@ -7,7 +7,7 @@ use accesskit::{
     NodeId, Point, Rect, Role, TextDirection, TextPosition as WeakPosition, TextSelection,
 };
 use alloc::{string::String, vec::Vec};
-use core::{cmp::Ordering, iter::FusedIterator};
+use core::{cmp::Ordering, fmt, iter::FusedIterator};
 
 use crate::{FilterResult, Node, TreeState};
 
@@ -549,7 +549,12 @@ impl<'a> Range<'a> {
 
     pub fn text(&self) -> String {
         let mut result = String::new();
-        self.walk::<_, ()>(|node| {
+        self.write_text(&mut result).unwrap();
+        result
+    }
+
+    pub fn write_text<W: fmt::Write>(&self, mut writer: W) -> fmt::Result {
+        if let Some(err) = self.walk(|node| {
             let character_lengths = node.data().character_lengths();
             let start_index = if node.id() == self.start.node.id() {
                 self.start.character_index
@@ -580,10 +585,12 @@ impl<'a> Range<'a> {
                         .sum::<usize>();
                 &value[slice_start..slice_end]
             };
-            result.push_str(s);
-            None
-        });
-        result
+            writer.write_str(s).err()
+        }) {
+            Err(err)
+        } else {
+            Ok(())
+        }
     }
 
     /// Returns the range's transformed bounding boxes relative to the tree's

platforms/windows/src/adapter.rs

@@ -111,11 +111,12 @@ impl TreeChangeHandler for AdapterChangeHandler<'_> {
         let old_wrapper = NodeWrapper(old_node);
         let new_wrapper = NodeWrapper(new_node);
         new_wrapper.enqueue_property_changes(&mut self.queue, &element, &old_wrapper);
-        if new_wrapper.name().is_some()
+        let new_name = new_wrapper.name();
+        if new_name.is_some()
             && new_node.live() != Live::Off
-            && (new_wrapper.name() != old_wrapper.name()
-                || new_node.live() != old_node.live()
-                || filter(old_node) != FilterResult::Include)
+            && (new_node.live() != old_node.live()
+                || filter(old_node) != FilterResult::Include
+                || new_name != old_wrapper.name())
         {
             self.queue.push(QueuedEvent::Simple {
                 element,

platforms/windows/src/node.rs

@@ -258,12 +258,15 @@ impl NodeWrapper<'_> {
         self.0.role_description()
     }
 
-    pub(crate) fn name(&self) -> Option<String> {
+    pub(crate) fn name(&self) -> Option<WideString> {
+        let mut result = WideString::default();
         if self.0.label_comes_from_value() {
-            self.0.value()
+            self.0.write_value(&mut result)
         } else {
-            self.0.label()
+            self.0.write_label(&mut result)
         }
+        .unwrap()
+        .then_some(result)
     }
 
     fn description(&self) -> Option<String> {
@@ -339,8 +342,10 @@ impl NodeWrapper<'_> {
         self.0.numeric_value().is_some()
     }
 
-    fn value(&self) -> String {
-        self.0.value().unwrap()
+    fn value(&self) -> WideString {
+        let mut result = WideString::default();
+        self.0.write_value(&mut result).unwrap();
+        result
     }
 
     fn is_read_only(&self) -> bool {

platforms/windows/src/text.rs

@@ -483,7 +483,11 @@ impl ITextRangeProvider_Impl for PlatformRange_Impl {
     fn GetText(&self, _max_length: i32) -> Result<BSTR> {
         // The Microsoft docs imply that the provider isn't _required_
         // to truncate text at the max length, so we just ignore it.
-        self.read(|range| Ok(range.text().into()))
+        self.read(|range| {
+            let mut result = WideString::default();
+            range.write_text(&mut result).unwrap();
+            Ok(result.into())
+        })
     }
 
     fn Move(&self, unit: TextUnit, count: i32) -> Result<i32> {

platforms/windows/src/util.rs

@@ -5,7 +5,10 @@
 
 use accesskit::Point;
 use accesskit_consumer::TreeState;
-use std::sync::{Arc, Weak};
+use std::{
+    fmt::{self, Write},
+    sync::{Arc, Weak},
+};
 use windows::{
     core::*,
     Win32::{
@@ -18,6 +21,27 @@ use windows::{
 
 use crate::window_handle::WindowHandle;
 
+#[derive(Clone, Default, PartialEq, Eq)]
+pub(crate) struct WideString(Vec<u16>);
+
+impl Write for WideString {
+    fn write_str(&mut self, s: &str) -> fmt::Result {
+        self.0.extend(s.encode_utf16());
+        Ok(())
+    }
+
+    fn write_char(&mut self, c: char) -> fmt::Result {
+        self.0.extend_from_slice(c.encode_utf16(&mut [0; 2]));
+        Ok(())
+    }
+}
+
+impl From<WideString> for BSTR {
+    fn from(value: WideString) -> Self {
+        Self::from_wide(&value.0).unwrap()
+    }
+}
+
 pub(crate) struct Variant(VARIANT);
 
 impl From<Variant> for VARIANT {
@@ -36,22 +60,29 @@ impl Variant {
     }
 }
 
-impl From<&str> for Variant {
-    fn from(value: &str) -> Self {
+impl From<BSTR> for Variant {
+    fn from(value: BSTR) -> Self {
         Self(value.into())
     }
 }
 
-impl From<String> for Variant {
-    fn from(value: String) -> Self {
-        let value: BSTR = value.into();
-        Self(value.into())
+impl From<WideString> for Variant {
+    fn from(value: WideString) -> Self {
+        BSTR::from(value).into()
     }
 }
 
-impl From<BSTR> for Variant {
-    fn from(value: BSTR) -> Self {
-        Self(value.into())
+impl From<&str> for Variant {
+    fn from(value: &str) -> Self {
+        let mut result = WideString::default();
+        result.write_str(value).unwrap();
+        result.into()
+    }
+}
+
+impl From<String> for Variant {
+    fn from(value: String) -> Self {
+        value.as_str().into()
     }
 }
 
@@ -216,13 +247,15 @@ pub(crate) fn window_title(hwnd: WindowHandle) -> Option<BSTR> {
     Some(BSTR::from_wide(&buffer).unwrap())
 }
 
-pub(crate) fn toolkit_description(state: &TreeState) -> Option<String> {
+pub(crate) fn toolkit_description(state: &TreeState) -> Option<WideString> {
     state.toolkit_name().map(|name| {
+        let mut result = WideString::default();
+        result.write_str(name).unwrap();
         if let Some(version) = state.toolkit_version() {
-            format!("{} {}", name, version)
-        } else {
-            name.to_string()
+            result.write_char(' ').unwrap();
+            result.write_str(version).unwrap();
         }
+        result
     })
 }