From c804db0f933b3454ba6e5c685a2ac7c8da21e45a Mon Sep 17 00:00:00 2001 From: Tim Kuehn Date: Thu, 19 Sep 2013 21:29:23 -0400 Subject: [PATCH 1/4] deactive profiler when not in use; use newtype structs for task chans --- src/components/gfx/render_task.rs | 13 ++---- src/components/main/servo.rc | 37 ++++------------- src/components/msg/constellation_msg.rs | 12 +----- src/components/script/dom/window.rs | 10 ++--- src/components/script/layout_interface.rs | 12 +----- src/components/script/script_task.rs | 13 +----- src/components/util/time.rs | 50 +++++++++++++++-------- 7 files changed, 55 insertions(+), 92 deletions(-) diff --git a/src/components/gfx/render_task.rs b/src/components/gfx/render_task.rs index fad3a76aae2..0ef051f1413 100644 --- a/src/components/gfx/render_task.rs +++ b/src/components/gfx/render_task.rs @@ -59,19 +59,12 @@ pub fn BufferRequest(screen_rect: Rect, page_rect: Rect) -> BufferReq } #[deriving(Clone)] -pub struct RenderChan { - chan: SharedChan>, -} - +pub struct RenderChan{chan:SharedChan>} impl RenderChan { pub fn new(chan: Chan>) -> RenderChan { - RenderChan { - chan: SharedChan::new(chan), - } - } - pub fn send(&self, msg: Msg) { - self.chan.send(msg); + RenderChan{chan:SharedChan::new(chan)} } + pub fn send(&self, msg: Msg) { self.chan.send(msg) } } struct RenderTask { diff --git a/src/components/main/servo.rc b/src/components/main/servo.rc index ff68e9fb29c..a06330973b2 100755 --- a/src/components/main/servo.rc +++ b/src/components/main/servo.rc @@ -47,7 +47,7 @@ use gfx::opts; use servo_net::image_cache_task::ImageCacheTask; use servo_net::resource_task::ResourceTask; -use servo_util::time::{Profiler, ProfilerChan, PrintMsg}; +use servo_util::time::{Profiler, ProfilerChan}; pub use gfx::opts::Opts; pub use gfx::text; @@ -56,7 +56,7 @@ use std::comm; #[cfg(not(test))] use std::os; use std::rt::rtio::RtioTimer; -use std::rt::io::timer::Timer; +use std::task::spawn_with; #[path="compositing/mod.rs"] pub mod compositing; @@ -127,38 +127,19 @@ fn start(argc: int, argv: **u8, crate_map: *u8) -> int { fn run(opts: Opts) { let (shutdown_port, shutdown_chan) = comm::stream(); - let (profiler_port, profiler_chan) = comm::stream(); - let (compositor_port, compositor_chan) = comm::stream(); + let (profiler_port, profiler_chan) = special_stream!(ProfilerChan); + let (compositor_port, compositor_chan) = special_stream!(CompositorChan); - let profiler_chan = ProfilerChan::new(profiler_chan); - Profiler::create(profiler_port); - do opts.profiler_period.map |&period| { - let profiler_chan = profiler_chan.clone(); - let period = (period * 1000f) as u64; - do spawn { - let mut tm = Timer::new().unwrap(); - loop { - tm.sleep(period); - profiler_chan.send(PrintMsg); - } - } - }; - let compositor_chan = CompositorChan::new(compositor_chan); - let profiler_chan_clone = profiler_chan.clone(); + Profiler::create(profiler_port, profiler_chan.clone(), opts.profiler_period); - let opts_clone = opts.clone(); - - do spawn { - let profiler_chan = profiler_chan_clone.clone(); - let compositor_chan = compositor_chan.clone(); - - let opts = &opts_clone.clone(); + do spawn_with((profiler_chan.clone(), compositor_chan, opts.clone())) + |(profiler_chan, compositor_chan, opts)| { + let opts = &opts; // Create a Servo instance. - let resource_task = ResourceTask(); let image_cache_task = ImageCacheTask(resource_task.clone()); - let constellation_chan = Constellation::start(compositor_chan.clone(), + let constellation_chan = Constellation::start(compositor_chan, opts, resource_task, image_cache_task, diff --git a/src/components/msg/constellation_msg.rs b/src/components/msg/constellation_msg.rs index 7724d53c429..1e61114e3b1 100644 --- a/src/components/msg/constellation_msg.rs +++ b/src/components/msg/constellation_msg.rs @@ -12,18 +12,10 @@ use geom::size::Size2D; use geom::rect::Rect; #[deriving(Clone)] -pub struct ConstellationChan { - chan: SharedChan, -} - +pub struct ConstellationChan(SharedChan); impl ConstellationChan { pub fn new(chan: Chan) -> ConstellationChan { - ConstellationChan { - chan: SharedChan::new(chan), - } - } - pub fn send(&self, msg: Msg) { - self.chan.send(msg); + ConstellationChan(SharedChan::new(chan)) } } diff --git a/src/components/script/dom/window.rs b/src/components/script/dom/window.rs index a7bae87a618..7c60da4d6c5 100644 --- a/src/components/script/dom/window.rs +++ b/src/components/script/dom/window.rs @@ -30,6 +30,7 @@ use std::int; use std::libc; use std::rt::rtio::RtioTimer; use std::rt::io::timer::Timer; +use std::task::spawn_with; use js::jsapi::JSVal; pub enum TimerControlMsg { @@ -198,21 +199,20 @@ impl Window { compositor: @ScriptListener, image_cache_task: ImageCacheTask) -> @mut Window { - let script_chan_clone = script_chan.clone(); let win = @mut Window { page: page, - script_chan: script_chan, + script_chan: script_chan.clone(), compositor: compositor, wrapper: WrapperCache::new(), timer_chan: { let (timer_port, timer_chan) = comm::stream::(); let id = page.id.clone(); - do spawn { + do spawn_with(script_chan) |script_chan| { loop { match timer_port.recv() { TimerMessage_Close => break, - TimerMessage_Fire(td) => script_chan_clone.chan.send(FireTimerMsg(id, td)), - TimerMessage_TriggerExit => script_chan_clone.chan.send(ExitMsg), + TimerMessage_Fire(td) => script_chan.send(FireTimerMsg(id, td)), + TimerMessage_TriggerExit => script_chan.send(ExitMsg), } } } diff --git a/src/components/script/layout_interface.rs b/src/components/script/layout_interface.rs index 47f9c1ac362..ce7e6b5bd36 100644 --- a/src/components/script/layout_interface.rs +++ b/src/components/script/layout_interface.rs @@ -111,17 +111,9 @@ pub struct Reflow { /// Encapsulates a channel to the layout task. #[deriving(Clone)] -pub struct LayoutChan { - chan: SharedChan, -} - +pub struct LayoutChan(SharedChan); impl LayoutChan { pub fn new(chan: Chan) -> LayoutChan { - LayoutChan { - chan: SharedChan::new(chan), - } - } - pub fn send(&self, msg: Msg) { - self.chan.send(msg); + LayoutChan(SharedChan::new(chan)) } } diff --git a/src/components/script/script_task.rs b/src/components/script/script_task.rs index 55311b5cf2a..01960196c81 100644 --- a/src/components/script/script_task.rs +++ b/src/components/script/script_task.rs @@ -83,20 +83,11 @@ pub struct NewLayoutInfo { /// Encapsulates external communication with the script task. #[deriving(Clone)] -pub struct ScriptChan { - /// The channel used to send messages to the script task. - chan: SharedChan, -} - +pub struct ScriptChan(SharedChan); impl ScriptChan { /// Creates a new script chan. pub fn new(chan: Chan) -> ScriptChan { - ScriptChan { - chan: SharedChan::new(chan) - } - } - pub fn send(&self, msg: ScriptMsg) { - self.chan.send(msg); + ScriptChan(SharedChan::new(chan)) } } diff --git a/src/components/util/time.rs b/src/components/util/time.rs index a847bef12c7..9b9f54139ef 100644 --- a/src/components/util/time.rs +++ b/src/components/util/time.rs @@ -3,27 +3,21 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ // Timing functions. -use extra::time::precise_time_ns; -use std::cell::Cell; use std::comm::{Port, SharedChan}; -use extra::sort::tim_sort; use std::iterator::AdditiveIterator; +use std::rt::io::timer::Timer; +use std::task::spawn_with; + +use extra::sort::tim_sort; +use extra::time::precise_time_ns; use extra::treemap::TreeMap; // front-end representation of the profiler used to communicate with the profiler #[deriving(Clone)] -pub struct ProfilerChan { - chan: SharedChan, -} - +pub struct ProfilerChan(SharedChan); impl ProfilerChan { pub fn new(chan: Chan) -> ProfilerChan { - ProfilerChan { - chan: SharedChan::new(chan), - } - } - pub fn send(&self, msg: ProfilerMsg) { - self.chan.send(msg); + ProfilerChan(SharedChan::new(chan)) } } @@ -101,11 +95,31 @@ pub struct Profiler { } impl Profiler { - pub fn create(port: Port) { - let port = Cell::new(port); - do spawn { - let mut profiler = Profiler::new(port.take()); - profiler.start(); + pub fn create(port: Port, chan: ProfilerChan, period: Option) { + match period { + Some(period) => { + let period = (period * 1000f) as u64; + do spawn { + let mut timer = Timer::new().unwrap(); + loop { + timer.sleep(period); + if !chan.try_send(PrintMsg) { + break; + } + } + } + // Spawn the profiler + do spawn_with(port) |port| { + let mut profiler = Profiler::new(port); + profiler.start(); + } + } + None => { + // no-op to handle profiler messages when the profiler is inactive + do spawn_with(port) |port| { + while port.try_recv().is_some() {} + } + } } } From 9df66ff0219b149f041c8a624b6561002345ea3d Mon Sep 17 00:00:00 2001 From: Tim Kuehn Date: Fri, 20 Sep 2013 00:13:10 -0400 Subject: [PATCH 2/4] workaround for broken cross-crate generic newtype structs add issue number for newtype struct issue --- src/components/gfx/render_task.rs | 23 +++++++++++++++++++---- src/components/main/constellation.rs | 2 +- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/components/gfx/render_task.rs b/src/components/gfx/render_task.rs index 0ef051f1413..170c8ad6158 100644 --- a/src/components/gfx/render_task.rs +++ b/src/components/gfx/render_task.rs @@ -58,13 +58,28 @@ pub fn BufferRequest(screen_rect: Rect, page_rect: Rect) -> BufferReq } } +// FIXME(rust#9155): this should be a newtype struct, but +// generic newtypes ICE when compiled cross-crate #[deriving(Clone)] -pub struct RenderChan{chan:SharedChan>} -impl RenderChan { +pub struct RenderChan { + chan: SharedChan>, +} +impl RenderChan { pub fn new(chan: Chan>) -> RenderChan { - RenderChan{chan:SharedChan::new(chan)} + RenderChan { + chan: SharedChan::new(chan), + } + } +} +impl GenericChan> for RenderChan { + fn send(&self, msg: Msg) { + assert!(self.try_send(msg), "RenderChan.send: render port closed") + } +} +impl GenericSmartChan> for RenderChan { + fn try_send(&self, msg: Msg) -> bool { + self.chan.try_send(msg) } - pub fn send(&self, msg: Msg) { self.chan.send(msg) } } struct RenderTask { diff --git a/src/components/main/constellation.rs b/src/components/main/constellation.rs index 555325b80c0..fd477790193 100644 --- a/src/components/main/constellation.rs +++ b/src/components/main/constellation.rs @@ -438,7 +438,7 @@ impl Constellation { if pipeline.subpage_id.expect("Constellation: child frame does not have a subpage id. This should not be possible.") == subpage_id { child_frame_tree.rect = Some(rect.clone()); - let Rect { size: Size2D { width, height }, _ } = rect; + let Size2D { width, height } = rect.size; pipeline.script_chan.send(ResizeMsg(pipeline.id.clone(), Size2D { width: width as uint, height: height as uint From 84d731712fe47b78f9594f3ca7ee55a8996369f8 Mon Sep 17 00:00:00 2001 From: Tim Kuehn Date: Fri, 20 Sep 2013 02:16:54 -0400 Subject: [PATCH 3/4] cleaned up code in constellation to make it more maintainable --- src/components/main/constellation.rs | 77 +++++++++++++++------------- 1 file changed, 42 insertions(+), 35 deletions(-) diff --git a/src/components/main/constellation.rs b/src/components/main/constellation.rs index fd477790193..23230ca1991 100644 --- a/src/components/main/constellation.rs +++ b/src/components/main/constellation.rs @@ -428,45 +428,52 @@ impl Constellation { debug!("Received frame rect %? from %?, %?", rect, pipeline_id, subpage_id); let mut already_sent = HashSet::new(); + // Returns true if a child frame tree's subpage id matches the given subpage id + let subpage_eq = |child_frame_tree: & &mut ChildFrameTree| { + child_frame_tree.frame_tree.pipeline.subpage_id.expect("Constellation: + child frame does not have a subpage id. This should not be possible.") + == subpage_id + }; + + // Update a child's frame rect and inform its script task of the change, + // if it hasn't been already. Optionally inform the compositor if + // resize happens immediately. + let update_child_rect = |child_frame_tree: &mut ChildFrameTree, is_active: bool| { + child_frame_tree.rect = Some(rect.clone()); + let pipeline = &child_frame_tree.frame_tree.pipeline; + if !already_sent.contains(&pipeline.id) { + let Size2D { width, height } = rect.size; + if is_active { + pipeline.script_chan.send(ResizeMsg(pipeline.id, Size2D { + width: width as uint, + height: height as uint + })); + self.compositor_chan.send(SetLayerClipRect(pipeline.id, rect)); + } else { + pipeline.script_chan.send(ResizeInactiveMsg(pipeline.id, + Size2D(width as uint, height as uint))); + } + already_sent.insert(pipeline.id); + } + }; + // If the subframe is in the current frame tree, the compositor needs the new size for current_frame in self.current_frame().iter() { debug!("Constellation: Sending size for frame in current frame tree."); let source_frame = current_frame.find_mut(pipeline_id); for source_frame in source_frame.iter() { - for child_frame_tree in source_frame.children.mut_iter() { - let pipeline = &child_frame_tree.frame_tree.pipeline; - if pipeline.subpage_id.expect("Constellation: child frame does not have a - subpage id. This should not be possible.") == subpage_id { - child_frame_tree.rect = Some(rect.clone()); - let Size2D { width, height } = rect.size; - pipeline.script_chan.send(ResizeMsg(pipeline.id.clone(), Size2D { - width: width as uint, - height: height as uint - })); - self.compositor_chan.send(SetLayerClipRect(pipeline.id, rect)); - already_sent.insert(pipeline.id.clone()); - break; - } - } + let found_child = source_frame.children.mut_iter() + .find(|child| subpage_eq(child)); + found_child.map_move(|child| update_child_rect(child, true)); } } - // Traverse the navigation context and pending frames and tell each associated pipeline to resize. + + // Update all frames with matching pipeline- and subpage-ids let frames = self.find_all(pipeline_id); for frame_tree in frames.iter() { - for child_frame_tree in frame_tree.children.mut_iter() { - let pipeline = &child_frame_tree.frame_tree.pipeline; - if pipeline.subpage_id.expect("Constellation: child frame does not have a - subpage id. This should not be possible.") == subpage_id { - child_frame_tree.rect = Some(rect.clone()); - if !already_sent.contains(&pipeline.id) { - let Size2D { width, height } = rect.size; - pipeline.script_chan.send(ResizeInactiveMsg(pipeline.id.clone(), - Size2D(width as uint, height as uint))); - already_sent.insert(pipeline.id.clone()); - } - break; - } - } + let found_child = frame_tree.children.mut_iter() + .find(|child| subpage_eq(child)); + found_child.map_move(|child| update_child_rect(child, false)); } // At this point, if no pipelines were sent a resize msg, then this subpage id @@ -588,7 +595,7 @@ impl Constellation { // changes would be overriden by changing the subframe associated with source_id. let parent = source_frame.parent.clone(); - let subpage_id = source_frame.pipeline.subpage_id.clone(); + let subpage_id = source_frame.pipeline.subpage_id; let next_pipeline_id = self.get_next_pipeline_id(); let pipeline = @mut Pipeline::create(next_pipeline_id, @@ -743,15 +750,15 @@ impl Constellation { fn handle_resized_window_msg(&mut self, new_size: Size2D) { let mut already_seen = HashSet::new(); for &@FrameTree { pipeline: pipeline, _ } in self.current_frame().iter() { - pipeline.script_chan.send(ResizeMsg(pipeline.id.clone(), new_size)); - already_seen.insert(pipeline.id.clone()); + pipeline.script_chan.send(ResizeMsg(pipeline.id, new_size)); + already_seen.insert(pipeline.id); } for frame_tree in self.navigation_context.previous.iter() .chain(self.navigation_context.next.iter()) { let pipeline = &frame_tree.pipeline; if !already_seen.contains(&pipeline.id) { - pipeline.script_chan.send(ResizeInactiveMsg(pipeline.id.clone(), new_size)); - already_seen.insert(pipeline.id.clone()); + pipeline.script_chan.send(ResizeInactiveMsg(pipeline.id, new_size)); + already_seen.insert(pipeline.id); } } } From 5f600f0ec03d84d511744c4a8043d1f9dc81fb80 Mon Sep 17 00:00:00 2001 From: Tim Kuehn Date: Fri, 20 Sep 2013 03:45:16 -0400 Subject: [PATCH 4/4] fix constellation being inundated with messages from script. script task sent RendererReadyMsg after every reflow. now, the renderer sends RendererReady at the appropriate time, and _only_ if it doesn't have paint permission. --- src/components/gfx/render_task.rs | 24 +++++++++++++----------- src/components/main/constellation.rs | 13 +++++-------- src/components/main/pipeline.rs | 2 ++ src/components/script/script_task.rs | 3 +-- 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/components/gfx/render_task.rs b/src/components/gfx/render_task.rs index 170c8ad6158..63374dd71d6 100644 --- a/src/components/gfx/render_task.rs +++ b/src/components/gfx/render_task.rs @@ -9,7 +9,7 @@ use azure::azure_hl::{B8G8R8A8, DrawTarget}; use display_list::DisplayList; use servo_msg::compositor_msg::{RenderListener, IdleRenderState, RenderingRenderState, LayerBuffer}; use servo_msg::compositor_msg::{LayerBufferSet, Epoch}; -use servo_msg::constellation_msg::PipelineId; +use servo_msg::constellation_msg::{ConstellationChan, PipelineId, RendererReadyMsg}; use font_context::FontContext; use geom::matrix2d::Matrix2D; use geom::size::Size2D; @@ -17,8 +17,8 @@ use geom::rect::Rect; use opts::Opts; use render_context::RenderContext; -use std::cell::Cell; use std::comm::{Chan, Port, SharedChan}; +use std::task::spawn_with; use extra::arc::Arc; use servo_util::time::{ProfilerChan, profile}; @@ -86,6 +86,7 @@ struct RenderTask { id: PipelineId, port: Port>, compositor: C, + constellation_chan: ConstellationChan, font_ctx: @mut FontContext, opts: Opts, @@ -110,24 +111,21 @@ impl RenderTask { pub fn create(id: PipelineId, port: Port>, compositor: C, + constellation_chan: ConstellationChan, opts: Opts, profiler_chan: ProfilerChan) { - let compositor = Cell::new(compositor); - let opts = Cell::new(opts); - let port = Cell::new(port); - let profiler_chan = Cell::new(profiler_chan); - do spawn { - let compositor = compositor.take(); + do spawn_with((port, compositor, constellation_chan, opts, profiler_chan)) + |(port, compositor, constellation_chan, opts, profiler_chan)| { + let share_gl_context = compositor.get_gl_context(); - let opts = opts.take(); - let profiler_chan = profiler_chan.take(); // FIXME: rust/#5967 let mut render_task = RenderTask { id: id, - port: port.take(), + port: port, compositor: compositor, + constellation_chan: constellation_chan, font_ctx: @mut FontContext::new(opts.render_backend.clone(), false, profiler_chan.clone()), @@ -155,6 +153,8 @@ impl RenderTask { if self.paint_permission { self.epoch.next(); self.compositor.set_layer_page_size(self.id, render_layer.size, self.epoch); + } else { + self.constellation_chan.send(RendererReadyMsg(self.id)); } self.render_layer = Some(render_layer); self.last_paint_msg = None; @@ -284,6 +284,8 @@ impl RenderTask { debug!("render_task: returning surface"); if self.paint_permission { self.compositor.paint(self.id, layer_buffer_set.clone(), self.epoch); + } else { + self.constellation_chan.send(RendererReadyMsg(self.id)); } debug!("caching paint msg"); self.last_paint_msg = Some(layer_buffer_set); diff --git a/src/components/main/constellation.rs b/src/components/main/constellation.rs index 23230ca1991..07b9601d641 100644 --- a/src/components/main/constellation.rs +++ b/src/components/main/constellation.rs @@ -201,20 +201,18 @@ impl NavigationContext { pub fn back(&mut self) -> @mut FrameTree { self.next.push(self.current.take_unwrap()); self.current = Some(self.previous.pop()); - debug!("previous: %? next: %? current: %?", self.previous, self.next, *self.current.get_ref()); self.current.unwrap() } pub fn forward(&mut self) -> @mut FrameTree { self.previous.push(self.current.take_unwrap()); self.current = Some(self.next.pop()); - debug!("previous: %? next: %? current: %?", self.previous, self.next, *self.current.get_ref()); self.current.unwrap() } /// Loads a new set of page frames, returning all evicted frame trees pub fn load(&mut self, frame_tree: @mut FrameTree) -> ~[@mut FrameTree] { - debug!("navigating to %?", frame_tree); + debug!("navigating to %?", frame_tree.pipeline.id); let evicted = replace(&mut self.next, ~[]); if self.current.is_some() { self.previous.push(self.current.take_unwrap()); @@ -554,7 +552,7 @@ impl Constellation { if url.path.ends_with(".js") { pipeline.execute(url); } else { - debug!("Constellation: sending load msg to %?", pipeline); + debug!("Constellation: sending load msg to pipeline %?", pipeline.id); pipeline.load(url); } let rect = self.pending_sizes.pop(&(source_pipeline_id, subpage_id)); @@ -718,12 +716,11 @@ impl Constellation { // If to_add is not the root frame, then replace revoked_frame with it. // This conveniently keeps scissor rect size intact. - debug!("Constellation: replacing %? with %? in %?", revoke_id, to_add, next_frame_tree); + debug!("Constellation: replacing %? with %? in %?", + revoke_id, to_add.pipeline.id, next_frame_tree.pipeline.id); if to_add.parent.is_some() { - let replaced = next_frame_tree.replace_child(revoke_id, to_add); - debug!("Replaced child: %?", replaced); + next_frame_tree.replace_child(revoke_id, to_add); } - debug!("Constellation: frame tree after replacing: %?", next_frame_tree); } None => { diff --git a/src/components/main/pipeline.rs b/src/components/main/pipeline.rs index 37e8b1eb41e..b90edc7e320 100644 --- a/src/components/main/pipeline.rs +++ b/src/components/main/pipeline.rs @@ -54,6 +54,7 @@ impl Pipeline { RenderTask::create(id, render_port, compositor_chan.clone(), + constellation_chan.clone(), opts.clone(), profiler_chan.clone()); @@ -136,6 +137,7 @@ impl Pipeline { RenderTask::create(id, render_port, compositor_chan.clone(), + constellation_chan.clone(), opts.clone(), profiler_chan.clone()); diff --git a/src/components/script/script_task.rs b/src/components/script/script_task.rs index 01960196c81..8c800727657 100644 --- a/src/components/script/script_task.rs +++ b/src/components/script/script_task.rs @@ -21,7 +21,7 @@ use layout_interface::{ReflowDocumentDamage, ReflowForDisplay, ReflowGoal}; use layout_interface::ReflowMsg; use layout_interface; use servo_msg::constellation_msg::{ConstellationChan, LoadUrlMsg, NavigationDirection}; -use servo_msg::constellation_msg::{PipelineId, SubpageId, RendererReadyMsg}; +use servo_msg::constellation_msg::{PipelineId, SubpageId}; use servo_msg::constellation_msg::{LoadIframeUrlMsg, IFrameSandboxed, IFrameUnsandboxed}; use servo_msg::constellation_msg; @@ -582,7 +582,6 @@ impl ScriptTask { if page_tree.page.last_reflow_id == reflow_id { page_tree.page.layout_join_port = None; } - self.constellation_chan.send(RendererReadyMsg(pipeline_id)); self.compositor.set_ready_state(FinishedLoading); }