Skip to content

Commit

Permalink
Auto merge of #16037 - cbrewster:reduce_framestate_clones, r=asajeffrey
Browse files Browse the repository at this point in the history
Reduce cloning of FrameState

<!-- Please describe your changes on the following line: -->
This uses `kmerge` from itertools to construct the jsh future and past iterators without having to do a collect/sort. This allows us to reduce cloning of `FrameState`.

---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because refactoring

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16037)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Apr 7, 2017
2 parents 8c2548a + 5be1780 commit c87d0c6
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 22 deletions.
16 changes: 16 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions components/constellation/Cargo.toml
Expand Up @@ -21,6 +21,7 @@ euclid = "0.11"
gfx = {path = "../gfx"}
gfx_traits = {path = "../gfx_traits"}
ipc-channel = "0.7"
itertools = "0.5"
layout_traits = {path = "../layout_traits"}
log = "0.3.5"
msg = {path = "../msg"}
Expand Down
36 changes: 14 additions & 22 deletions components/constellation/constellation.rs
Expand Up @@ -81,6 +81,7 @@ use gfx_traits::Epoch;
use ipc_channel::{Error as IpcError};
use ipc_channel::ipc::{self, IpcSender, IpcReceiver};
use ipc_channel::router::ROUTER;
use itertools::Itertools;
use layout_traits::LayoutThreadFactory;
use log::{Log, LogLevel, LogLevelFilter, LogMetadata, LogRecord};
use msg::constellation_msg::{FrameId, FrameType, PipelineId};
Expand Down Expand Up @@ -109,6 +110,7 @@ use servo_rand::{Rng, SeedableRng, ServoRng, random};
use servo_remutex::ReentrantMutex;
use servo_url::{Host, ImmutableOrigin, ServoUrl};
use std::borrow::ToOwned;
use std::cmp::Ordering;
use std::collections::{HashMap, VecDeque};
use std::iter::once;
use std::marker::PhantomData;
Expand All @@ -117,7 +119,6 @@ use std::rc::{Rc, Weak};
use std::sync::Arc;
use std::sync::mpsc::{Receiver, Sender, channel};
use std::thread;
use std::time::Instant;
use style_traits::CSSPixel;
use style_traits::cursor::Cursor;
use style_traits::viewport::ViewportConstraints;
Expand Down Expand Up @@ -703,15 +704,10 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>

/// The joint session future is the merge of the session future of every
/// frame in the frame tree, sorted chronologically.
fn joint_session_future<'a>(&'a self, frame_id_root: FrameId) -> impl Iterator<Item=FrameState> {
let mut future: Vec<FrameState> = self.full_frame_tree_iter(frame_id_root)
.flat_map(|frame| frame.next.iter().cloned())
.collect();

// Sort the joint session future by the timestamp that the pipeline was navigated to
// in chronological order
future.sort_by(|a, b| a.instant.cmp(&b.instant));
future.into_iter()
fn joint_session_future<'a>(&'a self, frame_id_root: FrameId) -> impl Iterator<Item = &'a FrameState> + 'a {
self.full_frame_tree_iter(frame_id_root)
.map(|frame| frame.next.iter().rev())
.kmerge_by(|a, b| a.instant.cmp(&b.instant) == Ordering::Less)
}

/// Is the joint session future empty?
Expand All @@ -722,19 +718,15 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>

/// The joint session past is the merge of the session past of every
/// frame in the frame tree, sorted reverse chronologically.
fn joint_session_past<'a>(&self, frame_id_root: FrameId) -> impl Iterator<Item=FrameState> {
let mut past: Vec<(Instant, FrameState)> = self.full_frame_tree_iter(frame_id_root)
.flat_map(|frame| frame.prev.iter().rev().scan(frame.instant, |prev_instant, entry| {
fn joint_session_past<'a>(&'a self, frame_id_root: FrameId) -> impl Iterator<Item = &'a FrameState> + 'a {
self.full_frame_tree_iter(frame_id_root)
.map(|frame| frame.prev.iter().rev().scan(frame.instant, |prev_instant, entry| {
let instant = *prev_instant;
*prev_instant = entry.instant;
Some((instant, entry.clone()))
Some((instant, entry))
}))
.collect();

// Sort the joint session past by the timestamp that the pipeline was navigated from
// in reverse chronological order
past.sort_by(|a, b| b.0.cmp(&a.0));
past.into_iter().map(|(_, entry)| entry)
.kmerge_by(|a, b| a.0.cmp(&b.0) == Ordering::Greater)
.map(|(_, entry)| entry)
}

/// Is the joint session past empty?
Expand Down Expand Up @@ -1714,7 +1706,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
TraversalDirection::Forward(delta) => {
for entry in self.joint_session_future(top_level_frame_id).take(delta) {
size = size + 1;
table.insert(entry.frame_id, entry);
table.insert(entry.frame_id, entry.clone());
}
if size < delta {
return debug!("Traversing forward too much.");
Expand All @@ -1723,7 +1715,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
TraversalDirection::Back(delta) => {
for entry in self.joint_session_past(top_level_frame_id).take(delta) {
size = size + 1;
table.insert(entry.frame_id, entry);
table.insert(entry.frame_id, entry.clone());
}
if size < delta {
return debug!("Traversing back too much.");
Expand Down
1 change: 1 addition & 0 deletions components/constellation/lib.rs
Expand Up @@ -20,6 +20,7 @@ extern crate gaol;
extern crate gfx;
extern crate gfx_traits;
extern crate ipc_channel;
extern crate itertools;
extern crate layout_traits;
#[macro_use]
extern crate log;
Expand Down

0 comments on commit c87d0c6

Please sign in to comment.