Commit e8abebbc authored by shend's avatar shend Committed by Commit bot

Add heuristic to order non bit fields in ComputedStyleBase.

Currently, when we generate non bit fields, we order them in
alphabetical order. This may result in unnecessary padding, e.g.

struct {
  char a;
  /* char padding[3] */
  int b;
  char c;
  /* char padding[3] */
};

See [1] for an explanation.

This patch defines an ordering on field types so that we can sort
non bit fields from largest alignment size to smallest. This heuristic
minimises the padding most of the time:

struct {
  int b;
  char a;
  char c;
  /* char padding[2] */
};

This is purely a memory optimisation, not speed. It does not take into
account data access patterns and cache locality. We use the data from
[2] to define the order of types.

With this heuristic, some of the generated code has changed, but it is
unlikely to have changed the size of any structs.

Diff of generated files:
https://gist.github.com/darrnshn/81d146c2696ef100d56309e5b4b529ff/revisions

[1] http://www.drdobbs.com/cpp/padding-and-rearranging-structure-member/240007649?pgno=2
[2] https://codereview.chromium.org/2841413002

BUG=628043

Review-Url: https://codereview.chromium.org/2844223002
Cr-Commit-Position: refs/heads/master@{#468293}
parent 73fe6ba4
...@@ -17,6 +17,22 @@ from name_utilities import ( ...@@ -17,6 +17,22 @@ from name_utilities import (
from collections import defaultdict, OrderedDict from collections import defaultdict, OrderedDict
from itertools import chain from itertools import chain
# Heuristic ordering of types from largest to smallest, used to sort fields by their alignment sizes.
# Specifying the exact alignment sizes for each type is impossible because it's platform specific,
# so we define an ordering instead.
# The ordering comes from the data obtained in:
# https://codereview.chromium.org/2841413002
# TODO(shend): Put alignment sizes into code form, rather than linking to a CL which may disappear.
ALIGNMENT_ORDER = [
'double',
'FillLayer', 'BorderData', # Aligns like a void * (can be 32 or 64 bits)
'LengthBox', 'Length', 'float',
'StyleColor', 'Color', 'unsigned', 'int',
'short',
'char',
'bool'
]
# TODO(shend): Improve documentation and add docstrings. # TODO(shend): Improve documentation and add docstrings.
...@@ -267,14 +283,7 @@ def _create_fields(properties): ...@@ -267,14 +283,7 @@ def _create_fields(properties):
return fields return fields
def _reorder_fields(fields): def _reorder_bit_fields(bit_fields):
"""
Returns a list of fields ordered to minimise padding.
"""
# Separate out bit fields from non bit fields
bit_fields = [field for field in fields if field.is_bit_field]
non_bit_fields = [field for field in fields if not field.is_bit_field]
# Since fields cannot cross word boundaries, in order to minimize # Since fields cannot cross word boundaries, in order to minimize
# padding, group fields into buckets so that as many buckets as possible # padding, group fields into buckets so that as many buckets as possible
# are exactly 32 bits. Although this greedy approach may not always # are exactly 32 bits. Although this greedy approach may not always
...@@ -300,8 +309,28 @@ def _reorder_fields(fields): ...@@ -300,8 +309,28 @@ def _reorder_fields(fields):
if not added_to_bucket: if not added_to_bucket:
field_buckets.append([field]) field_buckets.append([field])
return _flatten_list(field_buckets)
def _reorder_non_bit_fields(non_bit_fields):
# A general rule of thumb is to sort members by their alignment requirement
# (from biggest aligned to smallest).
for field in non_bit_fields:
assert field.type_name in ALIGNMENT_ORDER, \
"Type {} has unknown alignment. Please update ALIGNMENT_ORDER to include it.".format(field.type_name)
return list(sorted(non_bit_fields, key=lambda f: ALIGNMENT_ORDER.index(f.type_name)))
def _reorder_fields(fields):
"""
Returns a list of fields ordered to minimise padding.
"""
# Separate out bit fields from non bit fields
bit_fields = [field for field in fields if field.is_bit_field]
non_bit_fields = [field for field in fields if not field.is_bit_field]
# Non bit fields go first, then the bit fields. # Non bit fields go first, then the bit fields.
return list(non_bit_fields) + _flatten_list(field_buckets) return _reorder_non_bit_fields(non_bit_fields) + _reorder_bit_fields(bit_fields)
class ComputedStyleBaseWriter(make_style_builder.StyleBuilderWriter): class ComputedStyleBaseWriter(make_style_builder.StyleBuilderWriter):
......
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