Skip to content

Commit

Permalink
Auto merge of #24148 - asajeffrey:script-prefetch, r=<try>
Browse files Browse the repository at this point in the history
Prefetch img, scripts and stylesheets during parsing

<!-- Please describe your changes on the following line: -->

Eagerly tokenize html input, and scan for `script` and `img` tags which can be prefetched into the cache. This allows resources to be loaded concurrently, which would otherwise be loaded sequentially due to being blocked by script execution. On the cloth webgl demo, this takes the time-to-paint down from 732ms to 560ms.

---
<!-- 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 #3847
- [x] These changes do not require tests because it's a perf improvement

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- 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/24148)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Sep 10, 2019
2 parents 6ed204d + ae50784 commit 32d32bf
Show file tree
Hide file tree
Showing 9 changed files with 372 additions and 62 deletions.
42 changes: 25 additions & 17 deletions components/net/http_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -881,9 +881,14 @@ fn http_network_or_cache_fetch(
// Step 5.9
match http_request.referrer {
Referrer::NoReferrer => (),
Referrer::ReferrerUrl(ref http_request_referrer) => http_request
.headers
.typed_insert::<Referer>(http_request_referrer.to_string().parse().unwrap()),
Referrer::ReferrerUrl(ref http_request_referrer) => {
if let Ok(referer) = http_request_referrer.to_string().parse::<Referer>() {
http_request.headers.typed_insert(referer);
} else {
// error!("Failed to parse {} as referer", http_request_referrer);
panic!("Failed to parse {} as referer", http_request_referrer);
}
},
Referrer::Client =>
// it should be impossible for referrer to be anything else during fetching
// https://fetch.spec.whatwg.org/#concept-request-referrer
Expand Down Expand Up @@ -946,20 +951,23 @@ fn http_network_or_cache_fetch(

// Step 5.16
let current_url = http_request.current_url();
let host = Host::from(
format!(
"{}{}",
current_url.host_str().unwrap(),
current_url
.port()
.map(|v| format!(":{}", v))
.unwrap_or("".into())
)
.parse::<Authority>()
.unwrap(),
);
if let Ok(host) = format!(
"{}{}",
current_url.host_str().unwrap(),
current_url
.port()
.map(|v| format!(":{}", v))
.unwrap_or("".into())
)
.parse::<Authority>()
.map(Host::from)
{
http_request.headers.typed_insert(host);
} else {
// error!("Failed to parse {} as authority", current_url);
panic!("Failed to parse {} as authority", current_url);
}

http_request.headers.typed_insert(host);
// unlike http_loader, we should not set the accept header
// here, according to the fetch spec
set_default_accept_encoding(&mut http_request.headers);
Expand Down Expand Up @@ -1484,7 +1492,7 @@ fn cors_preflight_fetch(
headers.sort();
let headers = headers
.iter()
.map(|name| HeaderName::from_str(name).unwrap())
.filter_map(|name| HeaderName::from_str(name).ok())
.collect::<Vec<HeaderName>>();

// Step 4
Expand Down
11 changes: 8 additions & 3 deletions components/net/resource_thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@ use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
use net_traits::request::{Destination, RequestBuilder};
use net_traits::response::{Response, ResponseInit};
use net_traits::storage_thread::StorageThreadMsg;
use net_traits::FetchTaskTarget;
use net_traits::WebSocketNetworkEvent;
use net_traits::{CookieSource, CoreResourceMsg, CoreResourceThread};
use net_traits::{CustomResponseMediator, FetchChannels};
use net_traits::{FetchResponseMsg, ResourceThreads, WebSocketDomAction};
use net_traits::{ResourceFetchTiming, ResourceTimingType};
use net_traits::{ResourceThreads, WebSocketDomAction};
use profile_traits::mem::ProfilerChan as MemProfilerChan;
use profile_traits::mem::{Report, ReportKind, ReportsChan};
use profile_traits::time::ProfilerChan;
Expand Down Expand Up @@ -245,6 +246,10 @@ impl ResourceChannelManager {
action_receiver,
http_state,
),
FetchChannels::Prefetch => {
self.resource_manager
.fetch(req_init, None, (), http_state, None)
},
},
CoreResourceMsg::DeleteCookies(request) => {
http_state
Expand Down Expand Up @@ -455,11 +460,11 @@ impl CoreResourceManager {
}
}

fn fetch(
fn fetch<Target: 'static + FetchTaskTarget + Send>(
&self,
request_builder: RequestBuilder,
res_init_: Option<ResponseInit>,
mut sender: IpcSender<FetchResponseMsg>,
mut sender: Target,
http_state: &Arc<HttpState>,
cancel_chan: Option<IpcReceiver<()>>,
) {
Expand Down
15 changes: 15 additions & 0 deletions components/net_traits/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,18 @@ impl FetchTaskTarget for IpcSender<FetchResponseMsg> {
}
}

impl FetchTaskTarget for () {
fn process_request_body(&mut self, _: &Request) {}

fn process_request_eof(&mut self, _: &Request) {}

fn process_response(&mut self, _: &Response) {}

fn process_response_chunk(&mut self, _: Vec<u8>) {}

fn process_response_eof(&mut self, _: &Response) {}
}

pub trait Action<Listener> {
fn process(self, listener: &mut Listener);
}
Expand Down Expand Up @@ -368,6 +380,9 @@ pub enum FetchChannels {
event_sender: IpcSender<WebSocketNetworkEvent>,
action_receiver: IpcReceiver<WebSocketDomAction>,
},
/// If the fetch is just being done to populate the cache,
/// not because the data is needed now.
Prefetch,
}

#[derive(Debug, Deserialize, Serialize)]
Expand Down
3 changes: 2 additions & 1 deletion components/script/dom/bindings/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ use msg::constellation_msg::{
use net_traits::filemanager_thread::RelativePos;
use net_traits::image::base::{Image, ImageMetadata};
use net_traits::image_cache::{ImageCache, PendingImageId};
use net_traits::request::{Request, RequestBuilder};
use net_traits::request::{Referrer, Request, RequestBuilder};
use net_traits::response::HttpsState;
use net_traits::response::{Response, ResponseBody};
use net_traits::storage_thread::StorageType;
Expand Down Expand Up @@ -456,6 +456,7 @@ unsafe_no_jsmanaged_fields!(Request);
unsafe_no_jsmanaged_fields!(RequestBuilder);
unsafe_no_jsmanaged_fields!(StyleSharedRwLock);
unsafe_no_jsmanaged_fields!(USVString);
unsafe_no_jsmanaged_fields!(Referrer);
unsafe_no_jsmanaged_fields!(ReferrerPolicy);
unsafe_no_jsmanaged_fields!(Response);
unsafe_no_jsmanaged_fields!(ResponseBody);
Expand Down
21 changes: 18 additions & 3 deletions components/script/dom/htmlimageelement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ use html5ever::{LocalName, Prefix};
use ipc_channel::ipc;
use ipc_channel::router::ROUTER;
use mime::{self, Mime};
use msg::constellation_msg::PipelineId;
use net_traits::image::base::{Image, ImageMetadata};
use net_traits::image_cache::UsePlaceholder;
use net_traits::image_cache::{CanRequestImages, ImageCache, ImageOrMetadataAvailable};
Expand All @@ -61,6 +62,7 @@ use net_traits::request::RequestBuilder;
use net_traits::{FetchMetadata, FetchResponseListener, FetchResponseMsg, NetworkError};
use net_traits::{ResourceFetchTiming, ResourceTimingType};
use num_traits::ToPrimitive;
use servo_url::origin::ImmutableOrigin;
use servo_url::origin::MutableOrigin;
use servo_url::ServoUrl;
use std::cell::{Cell, RefMut};
Expand Down Expand Up @@ -261,6 +263,17 @@ impl PreInvoke for ImageContext {
}
}

// This function is also used to prefetch an image in `script::dom::servoparser::prefetch`.
pub(crate) fn image_fetch_request(
img_url: ServoUrl,
origin: ImmutableOrigin,
pipeline_id: PipelineId,
) -> RequestBuilder {
RequestBuilder::new(img_url)
.origin(origin)
.pipeline_id(Some(pipeline_id))
}

impl HTMLImageElement {
/// Update the current image with a valid URL.
fn fetch_image(&self, img_url: &ServoUrl) {
Expand Down Expand Up @@ -326,9 +339,11 @@ impl HTMLImageElement {
}),
);

let request = RequestBuilder::new(img_url.clone())
.origin(document.origin().immutable().clone())
.pipeline_id(Some(document.global().pipeline_id()));
let request = image_fetch_request(
img_url.clone(),
document.origin().immutable().clone(),
document.global().pipeline_id(),
);

// This is a background load because the load blocker already fulfills the
// purpose of delaying the document's load event.
Expand Down
55 changes: 40 additions & 15 deletions components/script/dom/htmlscriptelement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,15 @@ use html5ever::{LocalName, Prefix};
use ipc_channel::ipc;
use ipc_channel::router::ROUTER;
use js::jsval::UndefinedValue;
use msg::constellation_msg::PipelineId;
use net_traits::request::{
CorsSettings, CredentialsMode, Destination, Referrer, RequestBuilder, RequestMode,
};
use net_traits::ReferrerPolicy;
use net_traits::{FetchMetadata, FetchResponseListener, Metadata, NetworkError};
use net_traits::{ResourceFetchTiming, ResourceTimingType};
use servo_atoms::Atom;
use servo_url::ImmutableOrigin;
use servo_url::ServoUrl;
use std::cell::Cell;
use std::fs::File;
Expand Down Expand Up @@ -292,19 +295,18 @@ impl ResourceTimingListener for ClassicContext {

impl PreInvoke for ClassicContext {}

/// <https://html.spec.whatwg.org/multipage/#fetch-a-classic-script>
fn fetch_a_classic_script(
script: &HTMLScriptElement,
kind: ExternalScriptKind,
/// Steps 1-2 of <https://html.spec.whatwg.org/multipage/#fetch-a-classic-script>
// This function is also used to prefetch a script in `script::dom::servoparser::prefetch`.
pub(crate) fn script_fetch_request(
url: ServoUrl,
cors_setting: Option<CorsSettings>,
origin: ImmutableOrigin,
pipeline_id: PipelineId,
referrer: Referrer,
referrer_policy: Option<ReferrerPolicy>,
integrity_metadata: String,
character_encoding: &'static Encoding,
) {
let doc = document_from_node(script);

// Step 1, 2.
let request = RequestBuilder::new(url.clone())
) -> RequestBuilder {
RequestBuilder::new(url)
.destination(Destination::Script)
// https://html.spec.whatwg.org/multipage/#create-a-potential-cors-request
// Step 1
Expand All @@ -318,11 +320,34 @@ fn fetch_a_classic_script(
Some(CorsSettings::Anonymous) => CredentialsMode::CredentialsSameOrigin,
_ => CredentialsMode::Include,
})
.origin(doc.origin().immutable().clone())
.pipeline_id(Some(script.global().pipeline_id()))
.referrer(Some(Referrer::ReferrerUrl(doc.url())))
.referrer_policy(doc.get_referrer_policy())
.integrity_metadata(integrity_metadata);
.origin(origin)
.pipeline_id(Some(pipeline_id))
.referrer(Some(referrer))
.referrer_policy(referrer_policy)
.integrity_metadata(integrity_metadata)
}

/// <https://html.spec.whatwg.org/multipage/#fetch-a-classic-script>
fn fetch_a_classic_script(
script: &HTMLScriptElement,
kind: ExternalScriptKind,
url: ServoUrl,
cors_setting: Option<CorsSettings>,
integrity_metadata: String,
character_encoding: &'static Encoding,
) {
let doc = document_from_node(script);

// Step 1, 2.
let request = script_fetch_request(
url.clone(),
cors_setting,
doc.origin().immutable().clone(),
script.global().pipeline_id(),
Referrer::ReferrerUrl(doc.url()),
doc.get_referrer_policy(),
integrity_metadata,
);

// TODO: Step 3, Add custom steps to perform fetch

Expand Down
45 changes: 41 additions & 4 deletions components/script/dom/servoparser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ use tendril::stream::LossyDecoder;

mod async_html;
mod html;
mod prefetch;
mod xml;

#[dom_struct]
Expand Down Expand Up @@ -101,6 +102,10 @@ pub struct ServoParser {
aborted: Cell<bool>,
/// <https://html.spec.whatwg.org/multipage/#script-created-parser>
script_created_parser: bool,
/// We do a quick-and-dirty parse of the input looking for resources to prefetch
prefetch_tokenizer: DomRefCell<prefetch::Tokenizer>,
#[ignore_malloc_size_of = "Defined in html5ever"]
prefetch_input: DomRefCell<BufferQueue>,
}

#[derive(PartialEq)]
Expand Down Expand Up @@ -403,6 +408,8 @@ impl ServoParser {
script_nesting_level: Default::default(),
aborted: Default::default(),
script_created_parser: kind == ParserKind::ScriptCreated,
prefetch_tokenizer: DomRefCell::new(prefetch::Tokenizer::new(document)),
prefetch_input: DomRefCell::new(BufferQueue::new()),
}
}

Expand All @@ -425,20 +432,50 @@ impl ServoParser {
)
}

fn push_tendril_input_chunk(&self, chunk: StrTendril) {
if !chunk.is_empty() {
// Per https://github.com/whatwg/html/issues/1495
// stylesheets should not be loaded for documents
// without browsing contexts.
// https://github.com/whatwg/html/issues/1495#issuecomment-230334047
// suggests that no content should be preloaded in such a case.
// We're conservative, and only prefetch for documents
// with browsing contexts.
if self.document.browsing_context().is_some() {
// Push the chunk into the prefetch input stream,
// which is tokenized eagerly, to scan for resources
// to prefetch. If the user script uses `document.write()`
// to overwrite the network input, this prefetching may
// have been wasted, but in most cases it won't.
let mut prefetch_input = self.prefetch_input.borrow_mut();
prefetch_input.push_back(chunk.clone());
self.prefetch_tokenizer
.borrow_mut()
.feed(&mut *prefetch_input);
}
// Push the chunk into the network input stream,
// which is tokenized lazily.
self.network_input.borrow_mut().push_back(chunk);
}
}

fn push_bytes_input_chunk(&self, chunk: Vec<u8>) {
// For byte input, we convert it to text using the network decoder.
let chunk = self
.network_decoder
.borrow_mut()
.as_mut()
.unwrap()
.decode(chunk);
if !chunk.is_empty() {
self.network_input.borrow_mut().push_back(chunk);
}
self.push_tendril_input_chunk(chunk);
}

fn push_string_input_chunk(&self, chunk: String) {
self.network_input.borrow_mut().push_back(chunk.into());
// Convert the chunk to a tendril so cloning it isn't expensive.
// The input has already been decoded as a string, so doesn't need
// to be decoded by the network decoder again.
let chunk = StrTendril::from(chunk);
self.push_tendril_input_chunk(chunk);
}

fn parse_sync(&self) {
Expand Down
Loading

0 comments on commit 32d32bf

Please sign in to comment.