Made PipelineId non-zero, so optional ids take no more space.

Cargo.lock

@@ -1914,6 +1914,14 @@ dependencies = [
  "webrender_api 0.50.0 (git+https://github.com/servo/webrender)",
 ]
 
+[[package]]
+name = "msg_tests"
+version = "0.0.1"
+dependencies = [
+ "msg 0.0.1",
+ "size_of_test 0.0.1",
+]
+
 [[package]]
 name = "multistr"
 version = "0.5.1"
@@ -2792,6 +2800,7 @@ dependencies = [
  "libservo 0.0.1",
  "log 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "metrics_tests 0.0.1",
+ "msg_tests 0.0.1",
  "net_tests 0.0.1",
  "net_traits_tests 0.0.1",
  "plugin_compiletest 0.0.1",

components/compositing/compositor.rs

@@ -6,6 +6,7 @@ use CompositionPipeline;
 use SendableFrameTree;
 use compositor_thread::{CompositorProxy, CompositorReceiver};
 use compositor_thread::{InitialCompositorState, Msg, RenderListener};
+use core::nonzero::NonZero;
 use euclid::{Point2D, TypedPoint2D, TypedVector2D, ScaleFactor};
 use gfx_traits::Epoch;
 use gleam::gl;
@@ -61,7 +62,7 @@ impl ConvertPipelineIdFromWebRender for webrender_api::PipelineId {
     fn from_webrender(&self) -> PipelineId {
         PipelineId {
             namespace_id: PipelineNamespaceId(self.0),
-            index: PipelineIndex(self.1),
+            index: PipelineIndex(NonZero::new(self.1).expect("Webrender pipeline zero?")),
         }
     }
 }

components/compositing/lib.rs

@@ -4,7 +4,9 @@
 
 #![deny(unsafe_code)]
 #![feature(box_syntax)]
+#![feature(nonzero)]
 
+extern crate core;
 extern crate euclid;
 extern crate gfx_traits;
 extern crate gleam;

components/msg/Cargo.toml

@@ -13,5 +13,5 @@ path = "lib.rs"
 bitflags = "0.7"
 heapsize = "0.4"
 heapsize_derive = "0.1"
-serde = "1.0"
+serde = { version = "1.0.14", features = [ "unstable" ] }
 webrender_api = {git = "https://github.com/servo/webrender", features = ["ipc"]}

components/msg/constellation_msg.rs

@@ -5,6 +5,7 @@
 //! The high-level interface from script to constellation. Using this abstract interface helps
 //! reduce coupling between these two components.
 
+use core::nonzero::NonZero;
 use std::cell::Cell;
 use std::fmt;
 use webrender_api;
@@ -194,10 +195,9 @@ impl PipelineNamespace {
         });
     }
 
-    fn next_index(&mut self) -> u32 {
-        let result = self.index;
-        self.index = result + 1;
-        result
+    fn next_index(&mut self) -> NonZero<u32> {
+        self.index += 1;
+        NonZero::new(self.index).expect("pipeline id index wrapped!")
     }
 
     fn next_pipeline_id(&mut self) -> PipelineId {
@@ -220,8 +220,9 @@ thread_local!(pub static PIPELINE_NAMESPACE: Cell<Option<PipelineNamespace>> = C
 #[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, HeapSizeOf, Ord, PartialEq, PartialOrd, Serialize)]
 pub struct PipelineNamespaceId(pub u32);
 
-#[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, HeapSizeOf, Ord, PartialEq, PartialOrd, Serialize)]
-pub struct PipelineIndex(pub u32);
+#[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)]
+pub struct PipelineIndex(pub NonZero<u32>);
+known_heap_size!(0, PipelineIndex);
 
 #[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, HeapSizeOf, Ord, PartialEq, PartialOrd, Serialize)]
 pub struct PipelineId {
@@ -242,7 +243,7 @@ impl PipelineId {
     pub fn to_webrender(&self) -> webrender_api::PipelineId {
         let PipelineNamespaceId(namespace_id) = self.namespace_id;
         let PipelineIndex(index) = self.index;
-        webrender_api::PipelineId(namespace_id, index)
+        webrender_api::PipelineId(namespace_id, index.get())
     }
 
     pub fn root_scroll_node(&self) -> webrender_api::ClipId {
@@ -258,17 +259,18 @@ impl fmt::Display for PipelineId {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         let PipelineNamespaceId(namespace_id) = self.namespace_id;
         let PipelineIndex(index) = self.index;
-        write!(fmt, "({},{})", namespace_id, index)
+        write!(fmt, "({},{})", namespace_id, index.get())
     }
 }
 
-#[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, HeapSizeOf, Ord, PartialEq, PartialOrd, Serialize)]
-pub struct BrowsingContextIndex(pub u32);
+#[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)]
+pub struct BrowsingContextIndex(pub NonZero<u32>);
+known_heap_size!(0, BrowsingContextIndex);
 
 #[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, HeapSizeOf, Ord, PartialEq, PartialOrd, Serialize)]
 pub struct BrowsingContextId {
     pub namespace_id: PipelineNamespaceId,
-    pub index: BrowsingContextIndex
+    pub index: BrowsingContextIndex,
 }
 
 impl BrowsingContextId {
@@ -286,7 +288,7 @@ impl fmt::Display for BrowsingContextId {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         let PipelineNamespaceId(namespace_id) = self.namespace_id;
         let BrowsingContextIndex(index) = self.index;
-        write!(fmt, "({},{})", namespace_id, index)
+        write!(fmt, "({},{})", namespace_id, index.get())
     }
 }
 
@@ -336,9 +338,12 @@ impl PartialEq<BrowsingContextId> for TopLevelBrowsingContextId {
 
 // We provide ids just for unit testing.
 pub const TEST_NAMESPACE: PipelineNamespaceId = PipelineNamespaceId(1234);
-pub const TEST_PIPELINE_INDEX: PipelineIndex = PipelineIndex(5678);
+#[allow(unsafe_code)]
+pub const TEST_PIPELINE_INDEX: PipelineIndex = unsafe { PipelineIndex(NonZero::new_unchecked(5678)) };
 pub const TEST_PIPELINE_ID: PipelineId = PipelineId { namespace_id: TEST_NAMESPACE, index: TEST_PIPELINE_INDEX };
-pub const TEST_BROWSING_CONTEXT_INDEX: BrowsingContextIndex = BrowsingContextIndex(8765);
+#[allow(unsafe_code)]
+pub const TEST_BROWSING_CONTEXT_INDEX: BrowsingContextIndex =
+    unsafe { BrowsingContextIndex(NonZero::new_unchecked(8765)) };
 pub const TEST_BROWSING_CONTEXT_ID: BrowsingContextId =
     BrowsingContextId { namespace_id: TEST_NAMESPACE, index: TEST_BROWSING_CONTEXT_INDEX };
 

components/msg/lib.rs

@@ -2,11 +2,15 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+#![feature(const_fn)]
+#![feature(const_nonzero_new)]
+#![feature(nonzero)]
 #![deny(unsafe_code)]
 
 #[macro_use]
 extern crate bitflags;
-extern crate heapsize;
+extern crate core;
+#[macro_use] extern crate heapsize;
 #[macro_use] extern crate heapsize_derive;
 #[macro_use] extern crate serde;
 extern crate webrender_api;

ports/servo/Cargo.toml

@@ -18,6 +18,7 @@ compiletest_helper = {path = "../../tests/compiletest/helper"}
 gfx_tests = {path = "../../tests/unit/gfx"}
 layout_tests = {path = "../../tests/unit/layout"}
 metrics_tests = {path = "../../tests/unit/metrics"}
+msg_tests = {path = "../../tests/unit/msg"}
 net_tests = {path = "../../tests/unit/net"}
 net_traits_tests = {path = "../../tests/unit/net_traits"}
 plugin_compiletest = {path = "../../tests/compiletest/plugin"}

tests/unit/metrics/paint_time.rs

@@ -8,7 +8,7 @@ use gfx::display_list::{DisplayItem, DisplayList, ImageDisplayItem};
 use gfx_traits::Epoch;
 use ipc_channel::ipc;
 use metrics::{PaintTimeMetrics, ProfilerMetadataFactory};
-use msg::constellation_msg::{PipelineId, PipelineIndex, PipelineNamespaceId};
+use msg::constellation_msg::TEST_PIPELINE_ID;
 use net_traits::image::base::PixelFormat;
 use profile_traits::time::{ProfilerChan, TimerMetadata};
 use style::computed_values::image_rendering;
@@ -23,30 +23,22 @@ impl ProfilerMetadataFactory for DummyProfilerMetadataFactory {
 
 #[test]
 fn test_paint_metrics_construction() {
-    let pipeline_id = PipelineId {
-        namespace_id: PipelineNamespaceId(1),
-        index: PipelineIndex(1),
-    };
     let (sender, _) = ipc::channel().unwrap();
     let profiler_chan = ProfilerChan(sender);
     let (layout_sender, _) = ipc::channel().unwrap();
     let (script_sender, _) = ipc::channel().unwrap();
-    let paint_time_metrics = PaintTimeMetrics::new(pipeline_id, profiler_chan, layout_sender, script_sender);
+    let paint_time_metrics = PaintTimeMetrics::new(TEST_PIPELINE_ID, profiler_chan, layout_sender, script_sender);
     assert_eq!(paint_time_metrics.get_navigation_start(), None, "navigation start is None");
     assert_eq!(paint_time_metrics.get_first_paint(), None, "first paint is None");
     assert_eq!(paint_time_metrics.get_first_contentful_paint(), None, "first contentful paint is None");
 }
 
 fn test_common(display_list: &DisplayList, epoch: Epoch) -> PaintTimeMetrics {
-    let pipeline_id = PipelineId {
-        namespace_id: PipelineNamespaceId(1),
-        index: PipelineIndex(1),
-    };
     let (sender, _) = ipc::channel().unwrap();
     let profiler_chan = ProfilerChan(sender);
     let (layout_sender, _) = ipc::channel().unwrap();
     let (script_sender, _) = ipc::channel().unwrap();
-    let mut paint_time_metrics = PaintTimeMetrics::new(pipeline_id, profiler_chan, layout_sender, script_sender);
+    let mut paint_time_metrics = PaintTimeMetrics::new(TEST_PIPELINE_ID, profiler_chan, layout_sender, script_sender);
     let dummy_profiler_metadata_factory = DummyProfilerMetadataFactory {};
 
     paint_time_metrics.maybe_observe_paint_time(&dummy_profiler_metadata_factory,
@@ -81,12 +73,8 @@ fn test_first_paint_setter() {
 
 #[test]
 fn test_first_contentful_paint_setter() {
-    let pipeline_id = PipelineId {
-        namespace_id: PipelineNamespaceId(1),
-        index: PipelineIndex(1),
-    };
     let image = DisplayItem::Image(Box::new(ImageDisplayItem {
-        base: BaseDisplayItem::empty(pipeline_id),
+        base: BaseDisplayItem::empty(TEST_PIPELINE_ID),
         webrender_image: WebRenderImageInfo {
             width: 1,
             height: 1,

tests/unit/msg/Cargo.toml

@@ -0,0 +1,14 @@
+[package]
+name = "msg_tests"
+version = "0.0.1"
+authors = ["The Servo Project Developers"]
+license = "MPL-2.0"
+
+[lib]
+name = "msg_tests"
+path = "lib.rs"
+doctest = false
+
+[dependencies]
+msg = {path = "../../../components/msg"}
+size_of_test = {path = "../../../components/size_of_test"}

tests/unit/msg/lib.rs

@@ -0,0 +1,8 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+extern crate msg;
+#[macro_use] extern crate size_of_test;
+
+#[cfg(all(test, target_pointer_width = "64"))] mod size_of;

tests/unit/msg/size_of.rs

@@ -0,0 +1,14 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+use msg::constellation_msg::BrowsingContextId;
+use msg::constellation_msg::PipelineId;
+use msg::constellation_msg::TopLevelBrowsingContextId;
+
+size_of_test!(test_size_of_pipeline_id, PipelineId, 8);
+size_of_test!(test_size_of_optional_pipeline_id, Option<PipelineId>, 8);
+size_of_test!(test_size_of_browsing_context_id, BrowsingContextId, 8);
+size_of_test!(test_size_of_optional_browsing_context_id, Option<BrowsingContextId>, 8);
+size_of_test!(test_size_of_top_level_browsing_context_id, TopLevelBrowsingContextId, 8);
+size_of_test!(test_size_of_top_level_optional_browsing_context_id, Option<TopLevelBrowsingContextId>, 8);