Bug 1418806 - Try to allocate possible size for AnimationValueMap before composing. r?birtles
The EffectSet count does not exactly represent the count what we really need
for AnimationValueMap, but in most cases it matches. For example;
1) The element has two different keyframes animations
@keyframes anim1 {
to { opacity: 0; }
}
@keyframes anim2 {
to { transform: rotate(360deg); }
}
In this case the number matches.
2) The element has two animations but both keyframes have the same CSS property
@keyframes anim1 {
to { opacity: 0; }
}
@keyframes anim2 {
to { opacity: 0.1; }
}
In this case the number doesn't match, moreover it results more memory than we
ever needed, but this case is presumably less common.
3) The element has an animation having keyframes for two different CSS
properties.
@keyframes anim {
from { opacity: 0; transform: rotate(360deg); }
}
In this kind of cases, the number doesn't match. But even so, this patch
reduces the opportunities that the AnimationValueMap tries to allocate a new
memory (i.e. less opportunities on expanding the map).
Note that when the hash map is expanded, we do allocate a new RawTable with the
new size then replace the old one with the new one [1], so I believe this
change will reduce the crash rate to some extent.
[1] https://hg.mozilla.org/mozilla-central/file/15c95df467be/servo/components/hashglobe/src/hash_map.rs#l734
MozReview-Commit-ID: 6tcF9aqXh7a
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -662,16 +662,26 @@ Gecko_UpdateAnimations(RawGeckoElementBo
presContext->EffectCompositor()
->RequestRestyle(const_cast<Element*>(aElement),
pseudoType,
EffectCompositor::RestyleType::Standard,
EffectCompositor::CascadeLevel::Animations);
}
}
+size_t
+Gecko_GetAnimationEffectCount(RawGeckoElementBorrowed aElementOrPseudo)
+{
+ CSSPseudoElementType pseudoType =
+ GetPseudoTypeFromElementForAnimation(aElementOrPseudo);
+
+ EffectSet* effectSet = EffectSet::GetEffectSet(aElementOrPseudo, pseudoType);
+ return effectSet ? effectSet->Count() : 0;
+}
+
bool
Gecko_ElementHasAnimations(RawGeckoElementBorrowed aElement)
{
CSSPseudoElementType pseudoType =
GetPseudoTypeFromElementForAnimation(aElement);
return !!EffectSet::GetEffectSet(aElement, pseudoType);
}
--- a/layout/style/ServoBindings.h
+++ b/layout/style/ServoBindings.h
@@ -252,16 +252,17 @@ void Gecko_CopyAnimationNames(RawGeckoSt
RawGeckoStyleAnimationListBorrowed aSrc);
// This function takes an already addrefed nsAtom
void Gecko_SetAnimationName(mozilla::StyleAnimation* aStyleAnimation,
nsAtom* aAtom);
void Gecko_UpdateAnimations(RawGeckoElementBorrowed aElementOrPseudo,
ComputedStyleBorrowedOrNull aOldComputedValues,
ComputedStyleBorrowedOrNull aComputedValues,
mozilla::UpdateAnimationsTasks aTasks);
+size_t Gecko_GetAnimationEffectCount(RawGeckoElementBorrowed aElementOrPseudo);
bool Gecko_ElementHasAnimations(RawGeckoElementBorrowed aElementOrPseudo);
bool Gecko_ElementHasCSSAnimations(RawGeckoElementBorrowed aElementOrPseudo);
bool Gecko_ElementHasCSSTransitions(RawGeckoElementBorrowed aElementOrPseudo);
size_t Gecko_ElementTransitions_Length(RawGeckoElementBorrowed aElementOrPseudo);
nsCSSPropertyID Gecko_ElementTransitions_PropertyAt(
RawGeckoElementBorrowed aElementOrPseudo,
size_t aIndex);
RawServoAnimationValueBorrowedOrNull Gecko_ElementTransitions_EndValueAt(
--- a/servo/components/style/gecko/wrapper.rs
+++ b/servo/components/style/gecko/wrapper.rs
@@ -32,16 +32,17 @@ use gecko_bindings::bindings;
use gecko_bindings::bindings::{Gecko_ElementState, Gecko_GetDocumentLWTheme};
use gecko_bindings::bindings::{Gecko_GetLastChild, Gecko_GetNextStyleChild};
use gecko_bindings::bindings::{Gecko_SetNodeFlags, Gecko_UnsetNodeFlags};
use gecko_bindings::bindings::Gecko_ClassOrClassList;
use gecko_bindings::bindings::Gecko_ElementHasAnimations;
use gecko_bindings::bindings::Gecko_ElementHasCSSAnimations;
use gecko_bindings::bindings::Gecko_ElementHasCSSTransitions;
use gecko_bindings::bindings::Gecko_GetActiveLinkAttrDeclarationBlock;
+use gecko_bindings::bindings::Gecko_GetAnimationEffectCount;
use gecko_bindings::bindings::Gecko_GetAnimationRule;
use gecko_bindings::bindings::Gecko_GetExtraContentStyleDeclarations;
use gecko_bindings::bindings::Gecko_GetHTMLPresentationAttrDeclarationBlock;
use gecko_bindings::bindings::Gecko_GetStyleAttrDeclarationBlock;
use gecko_bindings::bindings::Gecko_GetUnvisitedLinkAttrDeclarationBlock;
use gecko_bindings::bindings::Gecko_GetVisitedLinkAttrDeclarationBlock;
use gecko_bindings::bindings::Gecko_IsSignificantChild;
use gecko_bindings::bindings::Gecko_MatchLang;
@@ -943,18 +944,26 @@ fn selector_flags_to_node_flags(flags: E
gecko_flags
}
fn get_animation_rule(
element: &GeckoElement,
cascade_level: CascadeLevel,
) -> Option<Arc<Locked<PropertyDeclarationBlock>>> {
use gecko_bindings::sugar::ownership::HasSimpleFFI;
+ use properties::longhands::ANIMATABLE_PROPERTY_COUNT;
+
+ // There's a very rough correlation between the number of effects
+ // (animations) on an element and the number of properties it is likely to
+ // animate, so we use that as an initial guess for the size of the
+ // AnimationValueMap in order to reduce the number of re-allocations needed.
+ let effect_count = unsafe { Gecko_GetAnimationEffectCount(element.0) };
// Also, we should try to reuse the PDB, to avoid creating extra rule nodes.
- let mut animation_values = AnimationValueMap::default();
+ let mut animation_values = AnimationValueMap::with_capacity_and_hasher(
+ effect_count.min(ANIMATABLE_PROPERTY_COUNT), Default::default());
if unsafe {
Gecko_GetAnimationRule(
element.0,
cascade_level,
AnimationValueMap::as_ffi_mut(&mut animation_values),
)
} {
let shared_lock = &GLOBAL_STYLE_DATA.shared_lock;