Chromium Code Reviews Chromium Code Reviews
Issue
16161005:
Reduce CSSProperty's StylePropertyMetadata memory footprint by half when used inside a ImmutableSty… (Closed)
DescriptionReduce CSSProperty's StylePropertyMetadata memory footprint by half when used inside a ImmutableStylePropertySet.
Today CSSProperty holds its metadata in the following way :
- m_propertyID: 14 bits to hold the propertyID it represents. We
use 14 bits because CSSPropertyIDs start at 1001.
- m_shorthandID: These bits are used only to store if this given property
was set part of a shorthand. For example if border-top-color was set
from border-color or border. It will be 0 if just the longhand alone
was set. Again 14 bits to hold the CSSPropertyID of this shorthand.
- m_important: 1 bit used to describe if the property is !important or not.
- m_implicit: 1 bit used to describe if the property was implicitly set
or not. For example border-color: red will set border-color-top/right/left/bottom
implicitly.
- m_inherited: 1 bit used to describe if the property value was inherited or not.
The proposal to decrease the memory footprint on CSSProperty's metadata
only stand when stored inside ImmutableStylePropertySet which uses a custom
way to allocate and lay out the StylePropertyMetadata and the CSSValues in
memory because the idea behind is that the content will not change.
The MutableStylePropertySet uses a regular vector to retrieve, remove
and modify the CSSProperties. ImmutableStylePropertySet is used by default
when parsing up until someone start to access the CSSOM like
div.style which will convert the immutable to a mutable set. It is also good
to note that a CSSProperty is created for every single statement inside a block
in a stylesheet so we do have quite a bunch around. Another consideration is
that the only client to the m_shorthandID is the inspector which uses it
to group the longhands into a shorthand drop down list.
The new proposal is the following one :
- Reduce m_propertyID to 10 bits by not starting the CSSPropertyIDs from
1001 but rather 0 (or 3 as two are hardcoded CSSPropertyInvalid and CSSPropertyVariable).
After previous refactor in https://src.chromium.org/viewvc/blink?revision=151975&view=revision
and https://src.chromium.org/viewvc/blink?view=revision&revision=151920 it is safe to make
this change.
- Use the fact that we statically know which longhand belong to which shorthand. So
we create a static mapping between longhands and shorthands.
Here is the new layout :
- m_propertyID : 10 bits. We have up until 1024 properties. We have today 399, it
will take many years to reach 1024. A little message will make sure that when
1024 is reached people will have to modify CSSProperty.
- m_isSetFromShorthand : 1 bit to set wether this property was set from a shorthand
or not. This is all we need to know. From the propertyID we can retrieve which shorthand
it was set from using the new code in StylePropertyShorthand.
- m_indexInShorthandsVector : 2 bits, unfortunately there are few longhands which belong to multiple
shorthands so we need to store which was this longhand was part at parsing time. Notice
that it does not store the CSSPropertyID of the matching shorthand but rather its position
in the vector of matching shorthands. CSSProperty::m_shorthandID() method make it transparent
for call sites and return the actual CSSPropertyID of the shorthand. So far 2 bits seems
enough as there is only few longhands with ambiguity and they belong to 3 shorthands.
StylePropertyShorthand diff adds lot of boilerplate code, it may sounds
scarry but it's temporary. In a following CL we'll make sure that
this entire file is generated from a .in just like StyleBuilder or
others. That .in will just store the description of a shorthand and its longhand
and we will generate the entire file. We now have all the requirements so we
can generate all of it.
Please note also that this patch does not modify yet ImmutableStylePropertySet
to take into account the new size of the metadatas. Again this will
come in a following CL to keep this change focused.
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=152481
Patch Set 1 #Patch Set 2 : #
Total comments: 2
Patch Set 3 : #
Total comments: 2
Patch Set 4 : #
Total comments: 2
Patch Set 5 : #
Messages
Total messages: 12 (0 generated)
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
