From 6de127a343eed0124f2a5739e498869e5c717550 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Mon, 7 Mar 2016 14:08:02 +0100 Subject: [PATCH 1/5] Move the ImageCacheThread's closure into a function. --- components/net/image_cache_thread.rs | 89 +++++++++++++++------------- 1 file changed, 47 insertions(+), 42 deletions(-) diff --git a/components/net/image_cache_thread.rs b/components/net/image_cache_thread.rs index 22797bda379..cda0f6bad00 100644 --- a/components/net/image_cache_thread.rs +++ b/components/net/image_cache_thread.rs @@ -3,7 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use immeta::load_from_buf; -use ipc_channel::ipc::{self, IpcSender}; +use ipc_channel::ipc::{self, IpcSender, IpcReceiver}; use ipc_channel::router::ROUTER; use net_traits::image::base::{Image, ImageMetadata, load_from_memory, PixelFormat}; use net_traits::image_cache_thread::ImageResponder; @@ -286,7 +286,51 @@ fn convert_format(format: PixelFormat) -> webrender_traits::ImageFormat { } impl ImageCache { - fn run(&mut self) { + fn run(resource_thread: ResourceThread, + webrender_api: Option, + ipc_command_receiver: IpcReceiver) { + // Preload the placeholder image, used when images fail to load. + let mut placeholder_path = resources_dir_path(); + placeholder_path.push("rippy.png"); + + let mut image_data = vec![]; + let result = File::open(&placeholder_path).and_then(|mut file| { + file.read_to_end(&mut image_data) + }); + let placeholder_image = result.ok().map(|_| { + let mut image = load_from_memory(&image_data).unwrap(); + if let Some(ref webrender_api) = webrender_api { + let format = convert_format(image.format); + let mut bytes = Vec::new(); + bytes.extend_from_slice(&*image.bytes); + image.id = Some(webrender_api.add_image(image.width, image.height, format, bytes)); + } + Arc::new(image) + }); + + // Ask the router to proxy messages received over IPC to us. + let cmd_receiver = ROUTER.route_ipc_receiver_to_new_mpsc_receiver(ipc_command_receiver); + + let (progress_sender, progress_receiver) = channel(); + let (decoder_sender, decoder_receiver) = channel(); + let mut cache = ImageCache { + cmd_receiver: cmd_receiver, + progress_sender: progress_sender, + progress_receiver: progress_receiver, + decoder_sender: decoder_sender, + decoder_receiver: decoder_receiver, + thread_pool: ThreadPool::new(4), + pending_loads: AllPendingLoads::new(), + completed_loads: HashMap::new(), + resource_thread: resource_thread, + placeholder_image: placeholder_image, + webrender_api: webrender_api, + }; + + cache.do_run(); + } + + fn do_run(&mut self) { let mut exit_sender: Option> = None; loop { @@ -551,48 +595,9 @@ impl ImageCache { pub fn new_image_cache_thread(resource_thread: ResourceThread, webrender_api: Option) -> ImageCacheThread { let (ipc_command_sender, ipc_command_receiver) = ipc::channel().unwrap(); - let (progress_sender, progress_receiver) = channel(); - let (decoder_sender, decoder_receiver) = channel(); spawn_named("ImageCacheThread".to_owned(), move || { - - // Preload the placeholder image, used when images fail to load. - let mut placeholder_path = resources_dir_path(); - placeholder_path.push("rippy.png"); - - let mut image_data = vec![]; - let result = File::open(&placeholder_path).and_then(|mut file| { - file.read_to_end(&mut image_data) - }); - let placeholder_image = result.ok().map(|_| { - let mut image = load_from_memory(&image_data).unwrap(); - if let Some(ref webrender_api) = webrender_api { - let format = convert_format(image.format); - let mut bytes = Vec::new(); - bytes.extend_from_slice(&*image.bytes); - image.id = Some(webrender_api.add_image(image.width, image.height, format, bytes)); - } - Arc::new(image) - }); - - // Ask the router to proxy messages received over IPC to us. - let cmd_receiver = ROUTER.route_ipc_receiver_to_new_mpsc_receiver(ipc_command_receiver); - - let mut cache = ImageCache { - cmd_receiver: cmd_receiver, - progress_sender: progress_sender, - progress_receiver: progress_receiver, - decoder_sender: decoder_sender, - decoder_receiver: decoder_receiver, - thread_pool: ThreadPool::new(4), - pending_loads: AllPendingLoads::new(), - completed_loads: HashMap::new(), - resource_thread: resource_thread, - placeholder_image: placeholder_image, - webrender_api: webrender_api, - }; - - cache.run(); + ImageCache::run(resource_thread, webrender_api, ipc_command_receiver) }); ImageCacheThread::new(ipc_command_sender) From 9c4f2265d09bb4b56739f1f29f246ab0b84e00c6 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Mon, 7 Mar 2016 14:13:31 +0100 Subject: [PATCH 2/5] Use the select! macro in ImageCacheThread. --- components/net/image_cache_thread.rs | 34 ++++++++++++---------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/components/net/image_cache_thread.rs b/components/net/image_cache_thread.rs index cda0f6bad00..e95284853bf 100644 --- a/components/net/image_cache_thread.rs +++ b/components/net/image_cache_thread.rs @@ -18,7 +18,7 @@ use std::fs::File; use std::io::Read; use std::mem; use std::sync::Arc; -use std::sync::mpsc::{Receiver, Select, Sender, channel}; +use std::sync::mpsc::{Receiver, Sender, channel}; use url::Url; use util::resource_files::resources_dir_path; use util::thread::spawn_named; @@ -335,26 +335,20 @@ impl ImageCache { loop { let result = { - let sel = Select::new(); + let cmd_receiver = &self.cmd_receiver; + let progress_receiver = &self.progress_receiver; + let decoder_receiver = &self.decoder_receiver; - let mut cmd_handle = sel.handle(&self.cmd_receiver); - let mut progress_handle = sel.handle(&self.progress_receiver); - let mut decoder_handle = sel.handle(&self.decoder_receiver); - - unsafe { - cmd_handle.add(); - progress_handle.add(); - decoder_handle.add(); - } - - let ret = sel.wait(); - - if ret == cmd_handle.id() { - SelectResult::Command(self.cmd_receiver.recv().unwrap()) - } else if ret == decoder_handle.id() { - SelectResult::Decoder(self.decoder_receiver.recv().unwrap()) - } else { - SelectResult::Progress(self.progress_receiver.recv().unwrap()) + select! { + msg = cmd_receiver.recv() => { + SelectResult::Command(msg.unwrap()) + }, + msg = decoder_receiver.recv() => { + SelectResult::Decoder(msg.unwrap()) + }, + msg = progress_receiver.recv() => { + SelectResult::Progress(msg.unwrap()) + } } }; From 9c75ddfe66558488a7d73b2e0b96034bf78ddaa5 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Mon, 7 Mar 2016 14:15:47 +0100 Subject: [PATCH 3/5] Inline ImageCache::do_run. --- components/net/image_cache_thread.rs | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/components/net/image_cache_thread.rs b/components/net/image_cache_thread.rs index e95284853bf..21c3044dd66 100644 --- a/components/net/image_cache_thread.rs +++ b/components/net/image_cache_thread.rs @@ -327,17 +327,13 @@ impl ImageCache { webrender_api: webrender_api, }; - cache.do_run(); - } - - fn do_run(&mut self) { let mut exit_sender: Option> = None; loop { let result = { - let cmd_receiver = &self.cmd_receiver; - let progress_receiver = &self.progress_receiver; - let decoder_receiver = &self.decoder_receiver; + let cmd_receiver = &cache.cmd_receiver; + let progress_receiver = &cache.progress_receiver; + let decoder_receiver = &cache.decoder_receiver; select! { msg = cmd_receiver.recv() => { @@ -354,19 +350,19 @@ impl ImageCache { match result { SelectResult::Command(cmd) => { - exit_sender = self.handle_cmd(cmd); + exit_sender = cache.handle_cmd(cmd); } SelectResult::Progress(msg) => { - self.handle_progress(msg); + cache.handle_progress(msg); } SelectResult::Decoder(msg) => { - self.handle_decoder(msg); + cache.handle_decoder(msg); } } // Can only exit when all pending loads are complete. if let Some(ref exit_sender) = exit_sender { - if self.pending_loads.is_empty() { + if cache.pending_loads.is_empty() { exit_sender.send(()).unwrap(); break; } From 1064c3dca01359e8bdbed7308ea266311113b5ab Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Mon, 7 Mar 2016 14:29:49 +0100 Subject: [PATCH 4/5] Move the receivers out of the ImageCache struct. --- components/net/image_cache_thread.rs | 36 ++++++++-------------------- 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/components/net/image_cache_thread.rs b/components/net/image_cache_thread.rs index 21c3044dd66..0e335c7ad99 100644 --- a/components/net/image_cache_thread.rs +++ b/components/net/image_cache_thread.rs @@ -18,7 +18,7 @@ use std::fs::File; use std::io::Read; use std::mem; use std::sync::Arc; -use std::sync::mpsc::{Receiver, Sender, channel}; +use std::sync::mpsc::{Sender, channel}; use url::Url; use util::resource_files::resources_dir_path; use util::thread::spawn_named; @@ -232,15 +232,8 @@ struct ResourceLoadInfo { /// Implementation of the image cache struct ImageCache { - // Receive commands from clients - cmd_receiver: Receiver, - - // Receive notifications from the resource thread - progress_receiver: Receiver, progress_sender: Sender, - // Receive notifications from the decoder thread pool - decoder_receiver: Receiver, decoder_sender: Sender, // Worker threads for decoding images. @@ -314,11 +307,8 @@ impl ImageCache { let (progress_sender, progress_receiver) = channel(); let (decoder_sender, decoder_receiver) = channel(); let mut cache = ImageCache { - cmd_receiver: cmd_receiver, progress_sender: progress_sender, - progress_receiver: progress_receiver, decoder_sender: decoder_sender, - decoder_receiver: decoder_receiver, thread_pool: ThreadPool::new(4), pending_loads: AllPendingLoads::new(), completed_loads: HashMap::new(), @@ -330,21 +320,15 @@ impl ImageCache { let mut exit_sender: Option> = None; loop { - let result = { - let cmd_receiver = &cache.cmd_receiver; - let progress_receiver = &cache.progress_receiver; - let decoder_receiver = &cache.decoder_receiver; - - select! { - msg = cmd_receiver.recv() => { - SelectResult::Command(msg.unwrap()) - }, - msg = decoder_receiver.recv() => { - SelectResult::Decoder(msg.unwrap()) - }, - msg = progress_receiver.recv() => { - SelectResult::Progress(msg.unwrap()) - } + let result = select! { + msg = cmd_receiver.recv() => { + SelectResult::Command(msg.unwrap()) + }, + msg = decoder_receiver.recv() => { + SelectResult::Decoder(msg.unwrap()) + }, + msg = progress_receiver.recv() => { + SelectResult::Progress(msg.unwrap()) } }; From d5b559c37ca9dbea9dfcb7969bdd96d4a77d7500 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Mon, 7 Mar 2016 14:30:20 +0100 Subject: [PATCH 5/5] Bonus fix: Rewrite get_image_or_meta_if_available in an imperative style. I believe this is more readable. --- components/net/image_cache_thread.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/components/net/image_cache_thread.rs b/components/net/image_cache_thread.rs index 0e335c7ad99..07868c4a4b3 100644 --- a/components/net/image_cache_thread.rs +++ b/components/net/image_cache_thread.rs @@ -554,12 +554,17 @@ impl ImageCache { } } None => { - self.pending_loads.get_by_url(&url).as_ref(). - map_or(Err(ImageState::NotRequested), |pl| pl.metadata.as_ref(). - map_or(Err(ImageState::Pending), |meta| - Ok(ImageOrMetadataAvailable::MetadataAvailable(meta.clone())) - ) - ) + let pl = match self.pending_loads.get_by_url(&url) { + Some(pl) => pl, + None => return Err(ImageState::NotRequested), + }; + + let meta = match pl.metadata { + Some(ref meta) => meta, + None => return Err(ImageState::Pending), + }; + + Ok(ImageOrMetadataAvailable::MetadataAvailable(meta.clone())) } } }