servo: Merge #11442 - Fragment debug_id u16 only exists in debug, prod will format mem address (from mitchhentges:87-debug-id); r=KiChjang
authorMitchell Hentges <mitchhentges@protonmail.com>
Sat, 04 Jun 2016 16:03:59 -0500
changeset 339013 44d5eb864a8c92a48cdafb6e2dd0f6889ded62ec
parent 339012 34a33f786aab45eeda4aaf63773a072c2700cd9b
child 339014 8bbda02c0e630f01498655d5c39bed0b11d7e54c
push id31307
push usergszorc@mozilla.com
push dateSat, 04 Feb 2017 00:59:06 +0000
treeherdermozilla-central@94079d43835f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersKiChjang
servo: Merge #11442 - Fragment debug_id u16 only exists in debug, prod will format mem address (from mitchhentges:87-debug-id); r=KiChjang <!-- Please describe your changes on the following line: --> Each fragment has a `u16` `debug_id` in debug mode, but no `debug_id` in production to save memory. To format a debug id in production, the address of the empty `debug_id` is displayed. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #87 (github issue number if applicable). <!-- Either: --> - [X] These changes do not require tests because it looks like it's not possible to mock out `cfg` options in `#[test]`s <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 5002dff85352a09ff79f0183e437876f36b77515
servo/components/layout/fragment.rs
servo/components/layout/layout_debug.rs
--- a/servo/components/layout/fragment.rs
+++ b/servo/components/layout/fragment.rs
@@ -17,16 +17,17 @@ use gfx;
 use gfx::display_list::{BLUR_INFLATION_FACTOR, OpaqueNode};
 use gfx::text::glyph::ByteIndex;
 use gfx::text::text_run::{TextRun, TextRunSlice};
 use gfx_traits::{FragmentType, LayerId, LayerType, StackingContextId};
 use incremental::{RECONSTRUCT_FLOW, RestyleDamage};
 use inline::{FIRST_FRAGMENT_OF_ELEMENT, InlineFragmentContext, InlineFragmentNodeInfo};
 use inline::{InlineMetrics, LAST_FRAGMENT_OF_ELEMENT};
 use ipc_channel::ipc::IpcSender;
+#[cfg(debug_assertions)]
 use layout_debug;
 use model::{self, IntrinsicISizes, IntrinsicISizesContribution, MaybeAuto, specified};
 use msg::constellation_msg::PipelineId;
 use net_traits::image::base::{Image, ImageMetadata};
 use net_traits::image_cache_thread::{ImageOrMetadataAvailable, UsePlaceholder};
 use range::*;
 use rustc_serialize::{Encodable, Encoder};
 use script::dom::htmlcanvaselement::HTMLCanvasData;
@@ -113,28 +114,30 @@ pub struct Fragment {
 
     /// The pseudo-element that this fragment represents.
     pub pseudo: PseudoElementType<()>,
 
     /// Various flags for this fragment.
     pub flags: FragmentFlags,
 
     /// A debug ID that is consistent for the life of this fragment (via transform etc).
-    pub debug_id: u16,
+    /// This ID should not be considered stable across multiple layouts or fragment
+    /// manipulations.
+    debug_id: DebugId,
 
     /// The ID of the StackingContext that contains this fragment. This is initialized
     /// to 0, but it assigned during the collect_stacking_contexts phase of display
     /// list construction.
     pub stacking_context_id: StackingContextId,
 }
 
 impl Encodable for Fragment {
     fn encode<S: Encoder>(&self, e: &mut S) -> Result<(), S::Error> {
         e.emit_struct("fragment", 0, |e| {
-            try!(e.emit_struct_field("id", 0, |e| self.debug_id().encode(e)));
+            try!(e.emit_struct_field("id", 0, |e| self.debug_id.encode(e)));
             try!(e.emit_struct_field("border_box", 1, |e| self.border_box.encode(e)));
             e.emit_struct_field("margin", 2, |e| self.margin.encode(e))
         })
     }
 }
 
 /// Info specific to the kind of fragment.
 ///
@@ -804,17 +807,17 @@ impl Fragment {
             restyle_damage: restyle_damage,
             border_box: LogicalRect::zero(writing_mode),
             border_padding: LogicalMargin::zero(writing_mode),
             margin: LogicalMargin::zero(writing_mode),
             specific: specific,
             inline_context: None,
             pseudo: node.get_pseudo_element_type().strip(),
             flags: FragmentFlags::empty(),
-            debug_id: layout_debug::generate_unique_debug_id(),
+            debug_id: DebugId::new(),
             stacking_context_id: StackingContextId::new(0),
         }
     }
 
     /// Constructs a new `Fragment` instance from an opaque node.
     pub fn from_opaque_node_and_style(node: OpaqueNode,
                                       pseudo: PseudoElementType<()>,
                                       style: Arc<ServoComputedValues>,
@@ -833,27 +836,21 @@ impl Fragment {
             restyle_damage: restyle_damage,
             border_box: LogicalRect::zero(writing_mode),
             border_padding: LogicalMargin::zero(writing_mode),
             margin: LogicalMargin::zero(writing_mode),
             specific: specific,
             inline_context: None,
             pseudo: pseudo,
             flags: FragmentFlags::empty(),
-            debug_id: layout_debug::generate_unique_debug_id(),
+            debug_id: DebugId::new(),
             stacking_context_id: StackingContextId::new(0),
         }
     }
 
-    /// Returns a debug ID of this fragment. This ID should not be considered stable across
-    /// multiple layouts or fragment manipulations.
-    pub fn debug_id(&self) -> u16 {
-        self.debug_id
-    }
-
     /// Transforms this fragment into another fragment of the given type, with the given size,
     /// preserving all the other data.
     pub fn transform(&self, size: LogicalSize<Au>, info: SpecificFragmentInfo)
                      -> Fragment {
         let new_border_box = LogicalRect::from_point_size(self.style.writing_mode,
                                                           self.border_box.start,
                                                           size);
 
@@ -867,17 +864,17 @@ impl Fragment {
             restyle_damage: restyle_damage,
             border_box: new_border_box,
             border_padding: self.border_padding,
             margin: self.margin,
             specific: info,
             inline_context: self.inline_context.clone(),
             pseudo: self.pseudo.clone(),
             flags: FragmentFlags::empty(),
-            debug_id: self.debug_id,
+            debug_id: self.debug_id.clone(),
             stacking_context_id: StackingContextId::new(0),
         }
     }
 
     /// Transforms this fragment using the given `SplitInfo`, preserving all the other data.
     pub fn transform_with_split_info(&self, split: &SplitInfo, text_run: Arc<TextRun>)
                                      -> Fragment {
         let size = LogicalSize::new(self.style.writing_mode,
@@ -2626,17 +2623,17 @@ impl fmt::Debug for Fragment {
         let damage_string = if self.restyle_damage != RestyleDamage::empty() {
             format!(" damage={:?}", self.restyle_damage)
         } else {
             "".to_owned()
         };
 
         write!(f, "{}({}) [{:?}] border_box={:?}{}{}{}",
             self.specific.get_type(),
-            self.debug_id(),
+            self.debug_id,
             self.specific,
             self.border_box,
             border_padding_string,
             margin_string,
             damage_string)
     }
 }
 
@@ -2782,8 +2779,58 @@ bitflags! {
 /// Specified distances from the margin edge of a block to its content in the inline direction.
 /// These are returned by `guess_inline_content_edge_offsets()` and are used in the float placement
 /// speculation logic.
 #[derive(Copy, Clone, Debug)]
 pub struct SpeculatedInlineContentEdgeOffsets {
     pub start: Au,
     pub end: Au,
 }
+
+#[cfg(not(debug_assertions))]
+#[derive(Clone)]
+struct DebugId;
+
+#[cfg(debug_assertions)]
+#[derive(Clone)]
+struct DebugId(u16);
+
+#[cfg(not(debug_assertions))]
+impl DebugId {
+    pub fn new() -> DebugId {
+        DebugId
+    }
+}
+
+#[cfg(debug_assertions)]
+impl DebugId {
+    pub fn new() -> DebugId {
+        DebugId(layout_debug::generate_unique_debug_id())
+    }
+}
+
+#[cfg(not(debug_assertions))]
+impl fmt::Display for DebugId {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "{:p}", &self)
+    }
+}
+
+#[cfg(debug_assertions)]
+impl fmt::Display for DebugId {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "{}", self.0)
+    }
+}
+
+#[cfg(not(debug_assertions))]
+impl Encodable for DebugId {
+    fn encode<S: Encoder>(&self, e: &mut S) -> Result<(), S::Error> {
+        e.emit_str(&format!("{:p}", &self))
+    }
+}
+
+#[cfg(debug_assertions)]
+impl Encodable for DebugId {
+    fn encode<S: Encoder>(&self, e: &mut S) -> Result<(), S::Error> {
+        e.emit_u16(self.0)
+    }
+}
--- a/servo/components/layout/layout_debug.rs
+++ b/servo/components/layout/layout_debug.rs
@@ -10,20 +10,22 @@
 
 use flow;
 use flow_ref::FlowRef;
 use rustc_serialize::json;
 use std::borrow::ToOwned;
 use std::cell::RefCell;
 use std::fs::File;
 use std::io::Write;
+#[cfg(debug_assertions)]
 use std::sync::atomic::{ATOMIC_USIZE_INIT, AtomicUsize, Ordering};
 
 thread_local!(static STATE_KEY: RefCell<Option<State>> = RefCell::new(None));
 
+#[cfg(debug_assertions)]
 static DEBUG_ID_COUNTER: AtomicUsize = ATOMIC_USIZE_INIT;
 
 pub struct Scope;
 
 #[macro_export]
 macro_rules! layout_debug_scope(
     ($($arg:tt)*) => (
         if cfg!(debug_assertions) {
@@ -91,16 +93,17 @@ impl Drop for Scope {
             }
         });
     }
 }
 
 /// Generate a unique ID. This is used for items such as Fragment
 /// which are often reallocated but represent essentially the
 /// same data.
+#[cfg(debug_assertions)]
 pub fn generate_unique_debug_id() -> u16 {
     DEBUG_ID_COUNTER.fetch_add(1, Ordering::SeqCst) as u16
 }
 
 /// Begin a layout debug trace. If this has not been called,
 /// creating debug scopes has no effect.
 pub fn begin_trace(flow_root: FlowRef) {
     assert!(STATE_KEY.with(|ref r| r.borrow().is_none()));