mirror of
https://github.com/servo/servo.git
synced 2025-08-03 04:30:10 +01:00
dom: Optimize IFrameCollection::validate (#38196)
The `IFrameCollection` was previously rebuilt too often. This PR tries to address the todo, to only rebuild the IFrameCollection when necessary. Testing: `CSS Selector Invalidation: :has() invalidation should not be O(n^2)` wpt-test changed status from timeout to failed. Fixes: #38131 (IFrameCollection::validate accounts for 30% of script time in DOM heavy scenarios) Co-authored-by: sharpshooter_pt <ibluegalaxy_taoj@163.com> Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>
This commit is contained in:
parent
03ab419793
commit
97f544aa20
4 changed files with 46 additions and 14 deletions
|
@ -3349,6 +3349,10 @@ impl Document {
|
|||
self.iframes.borrow_mut()
|
||||
}
|
||||
|
||||
pub(crate) fn invalidate_iframes_collection(&self) {
|
||||
self.iframes.borrow_mut().invalidate();
|
||||
}
|
||||
|
||||
pub(crate) fn get_dom_interactive(&self) -> Option<CrossProcessInstant> {
|
||||
self.dom_interactive.get()
|
||||
}
|
||||
|
@ -4145,7 +4149,7 @@ impl Document {
|
|||
scripts: Default::default(),
|
||||
anchors: Default::default(),
|
||||
applets: Default::default(),
|
||||
iframes: Default::default(),
|
||||
iframes: RefCell::new(IFrameCollection::new()),
|
||||
style_shared_lock: {
|
||||
/// Per-process shared lock for author-origin stylesheets
|
||||
///
|
||||
|
|
|
@ -43,7 +43,7 @@ use crate::dom::element::{
|
|||
use crate::dom::eventtarget::EventTarget;
|
||||
use crate::dom::globalscope::GlobalScope;
|
||||
use crate::dom::htmlelement::HTMLElement;
|
||||
use crate::dom::node::{Node, NodeDamage, NodeTraits, UnbindContext};
|
||||
use crate::dom::node::{BindContext, Node, NodeDamage, NodeTraits, UnbindContext};
|
||||
use crate::dom::trustedhtml::TrustedHTML;
|
||||
use crate::dom::virtualmethods::VirtualMethods;
|
||||
use crate::dom::windowproxy::WindowProxy;
|
||||
|
@ -813,6 +813,13 @@ impl VirtualMethods for HTMLIFrameElement {
|
|||
}
|
||||
}
|
||||
|
||||
fn bind_to_tree(&self, context: &BindContext, can_gc: CanGc) {
|
||||
if let Some(s) = self.super_type() {
|
||||
s.bind_to_tree(context, can_gc);
|
||||
}
|
||||
self.owner_document().invalidate_iframes_collection();
|
||||
}
|
||||
|
||||
fn unbind_from_tree(&self, context: &UnbindContext, can_gc: CanGc) {
|
||||
self.super_type().unwrap().unbind_from_tree(context, can_gc);
|
||||
|
||||
|
@ -865,5 +872,7 @@ impl VirtualMethods for HTMLIFrameElement {
|
|||
// a new iframe. Without this, the constellation gets very
|
||||
// confused.
|
||||
self.destroy_nested_browsing_context();
|
||||
|
||||
self.owner_document().invalidate_iframes_collection();
|
||||
}
|
||||
}
|
||||
|
|
|
@ -2,7 +2,6 @@
|
|||
* License, v. 2.0. If a copy of the MPL was not distributed with this
|
||||
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */
|
||||
|
||||
use std::cell::Cell;
|
||||
use std::default::Default;
|
||||
|
||||
use base::id::BrowsingContextId;
|
||||
|
@ -29,25 +28,31 @@ pub(crate) struct IFrame {
|
|||
#[derive(Default, JSTraceable, MallocSizeOf)]
|
||||
#[cfg_attr(crown, crown::unrooted_must_root_lint::must_root)]
|
||||
pub(crate) struct IFrameCollection {
|
||||
/// The version of the [`Document`] that this collection refers to. When the versions
|
||||
/// do not match, the collection will need to be rebuilt.
|
||||
document_version: Cell<u64>,
|
||||
/// The `<iframe>`s in the collection.
|
||||
iframes: Vec<IFrame>,
|
||||
/// When true, the collection will need to be rebuilt.
|
||||
invalid: bool,
|
||||
}
|
||||
|
||||
impl IFrameCollection {
|
||||
pub(crate) fn new() -> Self {
|
||||
Self {
|
||||
iframes: vec![],
|
||||
invalid: true,
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn invalidate(&mut self) {
|
||||
self.invalid = true;
|
||||
}
|
||||
|
||||
/// Validate that the collection is up-to-date with the given [`Document`]. If it isn't up-to-date
|
||||
/// rebuild it.
|
||||
pub(crate) fn validate(&mut self, document: &Document) {
|
||||
// TODO: Whether the DOM has changed at all can lead to rebuilding this collection
|
||||
// when it isn't necessary. A better signal might be if any `<iframe>` nodes have
|
||||
// been connected or disconnected.
|
||||
let document_node = DomRoot::from_ref(document.upcast::<Node>());
|
||||
let document_version = document_node.inclusive_descendants_version();
|
||||
if document_version == self.document_version.get() {
|
||||
if !self.invalid {
|
||||
return;
|
||||
}
|
||||
let document_node = DomRoot::from_ref(document.upcast::<Node>());
|
||||
|
||||
// Preserve any old sizes, but only for `<iframe>`s that already have a
|
||||
// BrowsingContextId and a set size.
|
||||
|
@ -75,7 +80,7 @@ impl IFrameCollection {
|
|||
}
|
||||
})
|
||||
.collect();
|
||||
self.document_version.set(document_version);
|
||||
self.invalid = false;
|
||||
}
|
||||
|
||||
pub(crate) fn get(&self, browsing_context_id: BrowsingContextId) -> Option<&IFrame> {
|
||||
|
|
|
@ -1,4 +1,18 @@
|
|||
[has-complexity.html]
|
||||
expected: TIMEOUT
|
||||
[Before appending 25000 elements]
|
||||
expected: FAIL
|
||||
|
||||
[After appending 25000 elements. This should not time out.]
|
||||
expected: FAIL
|
||||
|
||||
[After appending another 25000 elements. This should not time out.]
|
||||
expected: FAIL
|
||||
|
||||
[After appending div with 25000 elements. This should not time out.]
|
||||
expected: FAIL
|
||||
|
||||
[After removing div with 25000 elements. This should not time out.]
|
||||
expected: FAIL
|
||||
|
||||
[After removing 25000 elements one-by-one. This should not time out.]
|
||||
expected: FAIL
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue