Commit 51ad943c authored by shend's avatar shend Committed by Commit bot

Exploit sharing when comparing and copying groups in ComputedStyle.

Groups inside ComputedStyle, such as StyleSurroundData, are stored as
pointers. When we copy the StyleSurroundData of a ComputedStyle, instead
of copying each member inside StyleSurroundData, we can copy the pointer
itself and do a copy when we write to it (copy-on-write). Similarly,
when we compare two StyleSurroundData pointers, we can first check if
they're pointing to the same object. If so, then we know that the two
StyleSurroundDatas are equal without having to compare each member
inside StyleSurroundData.

This patch introduces two macros: fieldwise_compare and fieldwise_copy,
which generate the code to do this optimisation. These macros take a
a list of fields to compare/copy and generate the cheapest set of code
for that task, taking advantage of group sharing.

This patch removes the perf cost of:
https://codereview.chromium.org/2786883002

by generating the exact same code as what was originally handwritten.

Diff of generated code:
https://gist.github.com/darrnshn/fe97404e680016753030fc788fcf24ff/revisions

BUG=628043

Review-Url: https://codereview.chromium.org/2826633002
Cr-Commit-Position: refs/heads/master@{#467243}
parent 56432d95
......@@ -359,7 +359,7 @@ class ComputedStyleBaseWriter(make_style_builder.StyleBuilderWriter):
'ComputedStyleBaseConstants.h': self.generate_base_computed_style_constants,
}
@template_expander.use_jinja('ComputedStyleBase.h.tmpl')
@template_expander.use_jinja('ComputedStyleBase.h.tmpl', tests={'in': lambda a, b: a in b})
def generate_base_computed_style_h(self):
return {
'properties': self._properties,
......@@ -368,7 +368,7 @@ class ComputedStyleBaseWriter(make_style_builder.StyleBuilderWriter):
'computed_style': self._root_group,
}
@template_expander.use_jinja('ComputedStyleBase.cpp.tmpl')
@template_expander.use_jinja('ComputedStyleBase.cpp.tmpl', tests={'in': lambda a, b: a in b})
def generate_base_computed_style_cpp(self):
return {
'properties': self._properties,
......
......@@ -36,7 +36,7 @@ sys.path.insert(1, os.path.join(_current_dir, *([os.pardir] * 4)))
import jinja2
def apply_template(path_to_template, params, filters=None):
def apply_template(path_to_template, params, filters=None, tests=None):
dirname, basename = os.path.split(path_to_template)
path_to_templates = os.path.join(_current_dir, 'templates')
jinja_env = jinja2.Environment(
......@@ -46,15 +46,17 @@ def apply_template(path_to_template, params, filters=None):
trim_blocks=True) # so don't need {%- -%} everywhere
if filters:
jinja_env.filters.update(filters)
if tests:
jinja_env.tests.update(tests)
template = jinja_env.get_template(basename)
return template.render(params)
def use_jinja(template_file_name, filters=None):
def use_jinja(template_file_name, filters=None, tests=None):
def real_decorator(generator):
def generator_internal(*args, **kwargs):
parameters = generator(*args, **kwargs)
return apply_template(template_file_name, parameters, filters=filters)
return apply_template(template_file_name, parameters, filters=filters, tests=tests)
generator_internal.func_name = generator.func_name
return generator_internal
return real_decorator
{% from 'macros.tmpl' import license %}
{% from 'fields/field.tmpl' import getter_expression, setter_expression %}
{% from 'fields/field.tmpl' import getter_expression, setter_expression, fieldwise_copy %}
{{license()}}
#include "core/ComputedStyleBase.h"
......@@ -22,18 +22,21 @@ struct SameSizeAsComputedStyleBase {
// ensure that the buckets are placed so that each takes up at most 1 word.
ASSERT_SIZE(ComputedStyleBase, SameSizeAsComputedStyleBase);
void ComputedStyleBase::InheritFrom(const ComputedStyleBase& inheritParent,
void ComputedStyleBase::InheritFrom(const ComputedStyleBase& other,
IsAtShadowBoundary isAtShadowBoundary) {
{% for field in computed_style.all_fields if field.is_inherited %}
{{setter_expression(field)}} = inheritParent.{{getter_expression(field)}};
{% endfor %}
{{fieldwise_copy(computed_style, computed_style.all_fields
|selectattr("is_inherited")
|list
)|indent(2)}}
}
void ComputedStyleBase::CopyNonInheritedFromCached(
const ComputedStyleBase& other) {
{% for field in computed_style.all_fields if (field.is_property and not field.is_inherited) or field.is_inherited_flag %}
{{setter_expression(field)}} = other.{{getter_expression(field)}};
{% endfor %}
{{fieldwise_copy(computed_style, computed_style.all_fields
|rejectattr("is_nonproperty")
|rejectattr("is_inherited")
|list
)|indent(2)}}
}
void ComputedStyleBase::PropagateIndependentInheritedProperties(
......
{% from 'macros.tmpl' import license, print_if %}
{% from 'fields/field.tmpl' import encode, getter_expression, declare_storage %}
{% from 'fields/field.tmpl' import encode, getter_expression, declare_storage, fieldwise_compare %}
{% from 'fields/group.tmpl' import define_field_group_class %}
{{license()}}
......@@ -38,17 +38,23 @@ class CORE_EXPORT ComputedStyleBase {
public:
inline bool IndependentInheritedEqual(const ComputedStyleBase& o) const {
return (
{% for field in computed_style.all_fields if field.is_inherited and field.is_independent %}
{{getter_expression(field)}} == o.{{getter_expression(field)}}{{print_if(not loop.last, ' &&')}}
{% endfor %}
{{fieldwise_compare(computed_style, computed_style.all_fields
|selectattr("is_inherited")
|selectattr("is_independent")
|list
)|indent(8)}}
true
);
}
inline bool NonIndependentInheritedEqual(const ComputedStyleBase& o) const {
return (
{% for field in computed_style.all_fields if field.is_inherited and not field.is_independent %}
{{getter_expression(field)}} == o.{{getter_expression(field)}}{{print_if(not loop.last, ' &&')}}
{% endfor %}
{{fieldwise_compare(computed_style, computed_style.all_fields
|selectattr("is_inherited")
|rejectattr("is_independent")
|list
)|indent(8)}}
true
);
}
......@@ -58,9 +64,12 @@ class CORE_EXPORT ComputedStyleBase {
inline bool NonInheritedEqual(const ComputedStyleBase& o) const {
return (
{% for field in computed_style.all_fields if field.is_property and not field.is_inherited %}
{{getter_expression(field)}} == o.{{getter_expression(field)}}{{print_if(not loop.last, ' &&')}}
{% endfor %}
{{fieldwise_compare(computed_style, computed_style.all_fields
|selectattr("is_property")
|rejectattr("is_inherited")
|list
)|indent(8)}}
true
);
}
......
......@@ -53,3 +53,36 @@ unsigned {{field.name}} : {{field.size}}; // {{field.type_name}}
{{field.type_name}} {{field.name}};
{%- endif %}
{% endmacro %}
{# Given a group and a list of fields to compare, this generates a set of
equality comparisons on those fields. The generated comparisons take
advantage of group sharing. #}
{% macro fieldwise_compare(group, fields_to_compare) %}
{% for subgroup in group.subgroups %}
{# If every field in this subgroup is to be compared, we can compare the
group pointer instead. #}
{% if subgroup.all_fields|reject("in", fields_to_compare)|list|length == 0 -%}
{{subgroup.member_name}} == o.{{subgroup.member_name}} &&
{# Otherwise, we would have to recursively generate comparison operations
on fields in the subgroup. #}
{% elif subgroup.fields|select("in", fields_to_compare)|list|length > 0 -%}
{{fieldwise_compare(subgroup, fields_to_compare)}}
{% endif %}
{% endfor %}
{% for field in group.fields|select("in", fields_to_compare) -%}
{{getter_expression(field)}} == o.{{getter_expression(field)}} &&
{% endfor %}
{% endmacro %}
{% macro fieldwise_copy(group, fields_to_copy) %}
{% for subgroup in group.subgroups %}
{% if subgroup.all_fields|reject("in", fields_to_copy)|list|length == 0 -%}
{{subgroup.member_name}} = other.{{subgroup.member_name}};
{% elif subgroup.fields|select("in", fields_to_copy)|list|length > 0 -%}
{{fieldwise_copy(subgroup, fields_to_copy)}}
{% endif %}
{% endfor %}
{% for field in group.fields|select("in", fields_to_copy) -%}
{{setter_expression(field)}} = other.{{getter_expression(field)}};
{% endfor %}
{% endmacro %}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment