[librsvg: 6/7] Box the Element variant to make Text nodes smaller



commit 8312bc00b6a4abbac82fef0596fb25cad3a56eaf
Author: Federico Mena Quintero <federico gnome org>
Date:   Thu Mar 12 18:19:50 2020 -0600

    Box the Element variant to make Text nodes smaller
    
    We had this:
    
    enum NodeData {
        Element(Element),
        Text(NodeChars),
    }
    
    The size of that enum is the maximum of the size of Element and
    NodeChar, plus the enum discriminant.
    
    However, Element is *much* larger than NodeChars!  Element is around
    1696 bytes, and a large part of that is from the style
    structures (ComputedValues/SpecifiedValues).
    
    Now we have this:
    
    enum NodeData {
        Element(Box<Element>),
        Text(NodeChars),
    }
    
    So that the Element variant is just as big as a pointer (... plus an
    extra heap allocation in case the enum *is* in that variant).  Thus,
    for the Text case there is no extra overhead.
    
    This reduces the memory consumption from the file in issue #42 from
    800,501,568 bytes to 463,412,720 bytes.
    
    From valgrind --tool=massif and then ms_print:
    
    Before the change, where n=10 is the massif snapshot with peak memory
    consumption of 800,501,568 bytes.  Note that allocations are split
    more or less evenly at 46.03% between NodeData created through
    append_chars_to_parent() and create_node().
    
    --------------------------------------------------------------------------------
      n        time(i)         total(B)   useful-heap(B) extra-heap(B)    stacks(B)
    --------------------------------------------------------------------------------
     10  4,033,475,973      800,501,568      779,887,334    20,614,234            0
    97.42% (779,887,334B) (heap allocation functions) malloc/new/new[], --alloc-fns, etc.
    ->46.03% (368,466,840B) 0x49FE9C7: alloc (alloc.rs:84)
    | ->46.03% (368,466,840B) 0x49FE9C7: exchange_malloc (alloc.rs:206)
    |   ->46.03% (368,466,840B) 0x49FE9C7: 
new<core::cell::RefCell<rctree::NodeData<rsvg_internals::node::NodeData>>> (rc.rs:326)
    |     ->46.03% (368,466,840B) 0x49FE9C7: new<rsvg_internals::node::NodeData> (lib.rs:132)
    |       ->46.03% (368,466,840B) 0x49FE9C7: append_chars_to_parent (document.rs:483)
    |         ->46.03% (368,466,840B) 0x49FE9C7: rsvg_internals::document::DocumentBuilder::append_characters 
(document.rs:470)
    |           ->46.03% (368,466,840B) 0x4984C11: 
_ZN14rsvg_internals3xml8XmlState27element_creation_characters17hc4234dce5cf15e85E.llvm.10288825890258368314 
(xml.rs:346)
    |           ...
    |
    ->46.03% (368,433,000B) 0x499554D: alloc (alloc.rs:84)
    | ->46.03% (368,433,000B) 0x499554D: exchange_malloc (alloc.rs:206)
    |   ->46.03% (368,433,000B) 0x499554D: 
new<core::cell::RefCell<rctree::NodeData<rsvg_internals::node::NodeData>>> (rc.rs:326)
    |     ->46.03% (368,433,000B) 0x499554D: new<rsvg_internals::node::NodeData> (lib.rs:132)
    |       ->46.03% (368,433,000B) 0x499554D: rsvg_internals::create_node::creators::create_use 
(create_node.rs:49)
    |         ->46.03% (368,433,000B) 0x496A8FF: rsvg_internals::create_node::create_node (create_node.rs:267)
    |           ->46.03% (368,433,000B) 0x49FE259: rsvg_internals::document::DocumentBuilder::append_element 
(document.rs:436)
    |             ->46.03% (368,433,000B) 0x4984805: 
rsvg_internals::xml::XmlState::element_creation_start_element (xml.rs:321)
    
    After the change, where n=80 is the massif snapshot with peak memory
    consumption of 463,412,720 bytes.  Note how create_node() is now at
    76.1%, and append_characters() is down to 6.11%.
    
    --------------------------------------------------------------------------------
      n        time(i)         total(B)   useful-heap(B) extra-heap(B)    stacks(B)
    --------------------------------------------------------------------------------
     80 21,158,526,070      463,412,720      444,892,578    18,520,142            0
    96.00% (444,892,578B) (heap allocation functions) malloc/new/new[], --alloc-fns, etc.
    ->76.71% (355,474,496B) 0x497F504: alloc (alloc.rs:84)
    | ->76.71% (355,474,496B) 0x497F504: exchange_malloc (alloc.rs:206)
    |   ->76.71% (355,474,496B) 0x497F504: new<rsvg_internals::node::Element> (boxed.rs:121)
    |     ->76.71% (355,474,496B) 0x497F504: rsvg_internals::node::NodeData::new_element (node.rs:75)
    |       ->76.71% (355,461,840B) 0x4999E47: rsvg_internals::create_node::creators::create_use 
(create_node.rs:49)
    |       | ->76.71% (355,461,840B) 0x4969FEF: rsvg_internals::create_node::create_node (create_node.rs:267)
    |       |   ->76.71% (355,461,840B) 0x4A211B9: rsvg_internals::document::DocumentBuilder::append_element 
(document.rs:435)
    |       |     ->76.71% (355,461,840B) 0x4984D35: 
rsvg_internals::xml::XmlState::element_creation_start_element (xml.rs:320)
    |       ...
    |
    ->06.11% (28,312,272B) 0x4A21951: alloc (alloc.rs:84)
    | ->06.11% (28,312,272B) 0x4A21951: exchange_malloc (alloc.rs:206)
    |   ->06.11% (28,312,272B) 0x4A21951: 
new<core::cell::RefCell<rctree::NodeData<rsvg_internals::node::NodeData>>> (rc.rs:326)
    |     ->06.11% (28,312,272B) 0x4A21951: new<rsvg_internals::node::NodeData> (lib.rs:132)
    |       ->06.11% (28,312,272B) 0x4A21951: append_chars_to_parent (document.rs:482)
    |         ->06.11% (28,312,272B) 0x4A21951: rsvg_internals::document::DocumentBuilder::append_characters 
(document.rs:469)
    |           ->06.11% (28,312,272B) 0x4985141: 
_ZN14rsvg_internals3xml8XmlState27element_creation_characters17hc4234dce5cf15e85E.llvm.11242280878917811167 
(xml.rs:345)

 rsvg_internals/src/node.rs | 44 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 39 insertions(+), 5 deletions(-)
---
diff --git a/rsvg_internals/src/node.rs b/rsvg_internals/src/node.rs
index e44abffc..10469712 100644
--- a/rsvg_internals/src/node.rs
+++ b/rsvg_internals/src/node.rs
@@ -43,8 +43,42 @@ pub type RsvgNode = rctree::Node<NodeData>;
 /// See the [module documentation](index.html) for more information.
 pub type RsvgWeakNode = rctree::WeakNode<NodeData>;
 
+/// Data for a single DOM node.
+///
+/// ## Memory consumption
+///
+/// SVG files look like this, roughly:
+///
+/// ```xml
+/// <svg>
+///   <rect x="10" y="20"/>
+///   <path d="..."/>
+///   <text x="10" y="20">Hello</text>
+///   <!-- etc -->
+/// </svg>
+/// ```
+///
+/// Each element has a bunch of data, including the styles, which is
+/// the biggest consumer of memory within the `Element` struct.  But
+/// between each element there is a text node; in the example above
+/// there are a bunch of text nodes with just whitespace (newlines and
+/// spaces), and a single text node with "`Hello`" in it from the
+/// `<text>` element.
+///
+/// This enum uses `Box<Element>` instead of embedding the `Element`
+/// struct directly in its variant, in order to make text nodes as
+/// small as possible, i.e. without all the baggage in an `Element`.
+/// With the Box, the `Element` variant is the size of a pointer,
+/// which is smaller than the `Text` variant.
+///
+/// ## Accessing the node's contents
+///
+/// Code that traverses the DOM tree needs to find out at runtime what
+/// each node stands for.  First, use the `get_type` or `is_element`
+/// methods from the `NodeBorrow` trait to see if you can then call
+/// `borrow_chars`, `borrow_element`, or `borrow_element_mut`.
 pub enum NodeData {
-    Element(Element),
+    Element(Box<Element>),
     Text(NodeChars),
 }
 
@@ -72,7 +106,7 @@ impl NodeData {
         class: Option<&str>,
         node_impl: Box<dyn NodeTrait>,
     ) -> NodeData {
-        NodeData::Element(Element {
+        NodeData::Element(Box::new(Element {
             node_type,
             element_name: element_name.clone(),
             id: id.map(str::to_string),
@@ -85,7 +119,7 @@ impl NodeData {
             cond: true,
             style_attr: String::new(),
             node_impl,
-        })
+        }))
     }
 
     pub fn new_chars() -> NodeData {
@@ -531,14 +565,14 @@ impl NodeBorrow for RsvgNode {
 
     fn borrow_element(&self) -> Ref<Element> {
         Ref::map(self.borrow(), |n| match *n {
-            NodeData::Element(ref e) => e,
+            NodeData::Element(ref e) => e.as_ref(),
             _ => panic!("tried to borrow_element for a non-element node"),
         })
     }
 
     fn borrow_element_mut(&mut self) -> RefMut<Element> {
         RefMut::map(self.borrow_mut(), |n| match *n {
-            NodeData::Element(ref mut e) => e,
+            NodeData::Element(ref mut e) => e.as_mut(),
             _ => panic!("tried to borrow_element_mut for a non-element node"),
         })
     }


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]