Optimize sizeof(InlineBox) by eliminating class layout padding

Optimize sizeof(InlineBox) by eliminating padding

Considering that on 64-bit Windows allocations are done with 16 byte
granularity this reduces the per-instance memory usage of
InlineBox by 16 bytes - from 80 bytes to 64 bytes.

My estimate is that on a typical page where are 1K to 2K instances of
InlineBox and derived classes. And on text heavy page like Reddit topic
discussion I counted 6.7K instances. That means that the estimated memory
saving is anywhere from 16 KB to 100 KB per Renderer processes. 

Class layout before the change:

class blink::InlineBox [sizeof = 72]
 : public blink::DisplayItemClient {
  <padding> (4 bytes)
  [sizeof=4] blink::InlineBox::InlineBoxBitfields bitfields_
  <padding> (4 bytes)
  [sizeof=8] blink::InlineBox* next_
  [sizeof=8] blink::InlineBox* prev_
  [sizeof=8] blink::InlineFlowBox* parent_
  [sizeof=8] blink::LineLayoutItem line_layout_item_
  [sizeof=8] blink::LayoutPoint location_
  [sizeof=4] blink::LayoutUnit logical_width_
  <padding> (4 bytes)
}

class layout after the change:

class blink::InlineBox [sizeof = 64]
  : public blink::DisplayItemClient {
  <padding> (4 bytes)
  [sizeof=8] blink::InlineBox* next_
  [sizeof=8] blink::InlineBox* prev_
  [sizeof=8] blink::InlineFlowBox* parent_
  [sizeof=8] blink::LineLayoutItem line_layout_item_
  [sizeof=8] blink::LayoutPoint location_
  [sizeof=4] blink::LayoutUnit logical_width_
  [sizeof=4] blink::InlineBox::InlineBoxBitfields bitfields_
}

BUG=710933
Review-Url: https://codereview.chromium.org/2829683010
Cr-Commit-Position: refs/heads/master@{#466699}
Committed: https://chromium.googlesource.com/chromium/src/+/8a9eb48c6d24472e6812811ece2bf4a29d2c3747

[stanisc, 2017-04-21 01:19:32]

The CQ bit was checked by stanisc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-21 01:19:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[stanisc, 2017-04-21 01:31:21]

Description was changed from

==========
InlineBox padding

BUG=710933
==========

to

==========
InlineBox padding

Layout after the change:

class blink::InlineBox [sizeof = 64]
  : public blink::DisplayItemClient {
  <padding> (4 bytes)
  [sizeof=8] blink::InlineBox* next_
  [sizeof=8] blink::InlineBox* prev_
  [sizeof=8] blink::InlineFlowBox* parent_
  [sizeof=8] blink::LineLayoutItem line_layout_item_
  [sizeof=8] blink::LayoutPoint location_
  [sizeof=4] blink::LayoutUnit logical_width_
  [sizeof=4] blink::InlineBox::InlineBoxBitfields bitfields_
}

BUG=710933
==========

[commit-bot: I haz the power, 2017-04-21 02:55:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-21 02:55:42]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[stanisc, 2017-04-21 21:03:34]

Description was changed from

==========
InlineBox padding

Layout after the change:

class blink::InlineBox [sizeof = 64]
  : public blink::DisplayItemClient {
  <padding> (4 bytes)
  [sizeof=8] blink::InlineBox* next_
  [sizeof=8] blink::InlineBox* prev_
  [sizeof=8] blink::InlineFlowBox* parent_
  [sizeof=8] blink::LineLayoutItem line_layout_item_
  [sizeof=8] blink::LayoutPoint location_
  [sizeof=4] blink::LayoutUnit logical_width_
  [sizeof=4] blink::InlineBox::InlineBoxBitfields bitfields_
}

BUG=710933
==========

to

==========
Optimize sizeof(InlineBox) by eliminating padding

Considering that on 64-bit Windows allocations are done with 16 byte
granularity this reduces the per-instance memory usage of
InlineBox by 16 bytes - from 80 bytes to 64 bytes.

My estimate is that on a typical page where are 1K to 2K instances of
InlineBox and derived classes. And on text heavy page like Reddit topic
discussion I counted 6.7K instances. That means that the estimated memory
saving is anywhere from 16 KB to 100 KB per Renderer processes. 

Class layout before the change:

class blink::InlineBox [sizeof = 72]
 : public blink::DisplayItemClient {
  <padding> (4 bytes)
  [sizeof=4] blink::InlineBox::InlineBoxBitfields bitfields_
  <padding> (4 bytes)
  [sizeof=8] blink::InlineBox* next_
  [sizeof=8] blink::InlineBox* prev_
  [sizeof=8] blink::InlineFlowBox* parent_
  [sizeof=8] blink::LineLayoutItem line_layout_item_
  [sizeof=8] blink::LayoutPoint location_
  [sizeof=4] blink::LayoutUnit logical_width_
  <padding> (4 bytes)
}

class layout after the change:

class blink::InlineBox [sizeof = 64]
  : public blink::DisplayItemClient {
  <padding> (4 bytes)
  [sizeof=8] blink::InlineBox* next_
  [sizeof=8] blink::InlineBox* prev_
  [sizeof=8] blink::InlineFlowBox* parent_
  [sizeof=8] blink::LineLayoutItem line_layout_item_
  [sizeof=8] blink::LayoutPoint location_
  [sizeof=4] blink::LayoutUnit logical_width_
  [sizeof=4] blink::InlineBox::InlineBoxBitfields bitfields_
}

BUG=710933
==========

[stanisc, 2017-04-21 21:05:23]

stanisc@chromium.org changed reviewers:
+ chrishtr@chromium.org

[stanisc, 2017-04-21 21:05:24]

PTAL!

[mstensho (USE GERRIT), 2017-04-24 08:42:46]

mstensho@opera.com changed reviewers:
+ mstensho@opera.com

[mstensho (USE GERRIT), 2017-04-24 08:42:46]

lgtm

[stanisc, 2017-04-24 16:31:26]

The CQ bit was checked by stanisc@chromium.org

[commit-bot: I haz the power, 2017-04-24 16:33:01]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-24 18:51:28]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1493051486554800, "parent_rev":
"89c16baa8273f797ed026079633dbb80fd191c12", "commit_rev":
"8a9eb48c6d24472e6812811ece2bf4a29d2c3747"}

[commit-bot: I haz the power, 2017-04-24 18:51:41]

Description was changed from

==========
Optimize sizeof(InlineBox) by eliminating padding

Considering that on 64-bit Windows allocations are done with 16 byte
granularity this reduces the per-instance memory usage of
InlineBox by 16 bytes - from 80 bytes to 64 bytes.

My estimate is that on a typical page where are 1K to 2K instances of
InlineBox and derived classes. And on text heavy page like Reddit topic
discussion I counted 6.7K instances. That means that the estimated memory
saving is anywhere from 16 KB to 100 KB per Renderer processes. 

Class layout before the change:

class blink::InlineBox [sizeof = 72]
 : public blink::DisplayItemClient {
  <padding> (4 bytes)
  [sizeof=4] blink::InlineBox::InlineBoxBitfields bitfields_
  <padding> (4 bytes)
  [sizeof=8] blink::InlineBox* next_
  [sizeof=8] blink::InlineBox* prev_
  [sizeof=8] blink::InlineFlowBox* parent_
  [sizeof=8] blink::LineLayoutItem line_layout_item_
  [sizeof=8] blink::LayoutPoint location_
  [sizeof=4] blink::LayoutUnit logical_width_
  <padding> (4 bytes)
}

class layout after the change:

class blink::InlineBox [sizeof = 64]
  : public blink::DisplayItemClient {
  <padding> (4 bytes)
  [sizeof=8] blink::InlineBox* next_
  [sizeof=8] blink::InlineBox* prev_
  [sizeof=8] blink::InlineFlowBox* parent_
  [sizeof=8] blink::LineLayoutItem line_layout_item_
  [sizeof=8] blink::LayoutPoint location_
  [sizeof=4] blink::LayoutUnit logical_width_
  [sizeof=4] blink::InlineBox::InlineBoxBitfields bitfields_
}

BUG=710933
==========

to

==========
Optimize sizeof(InlineBox) by eliminating padding

Considering that on 64-bit Windows allocations are done with 16 byte
granularity this reduces the per-instance memory usage of
InlineBox by 16 bytes - from 80 bytes to 64 bytes.

My estimate is that on a typical page where are 1K to 2K instances of
InlineBox and derived classes. And on text heavy page like Reddit topic
discussion I counted 6.7K instances. That means that the estimated memory
saving is anywhere from 16 KB to 100 KB per Renderer processes. 

Class layout before the change:

class blink::InlineBox [sizeof = 72]
 : public blink::DisplayItemClient {
  <padding> (4 bytes)
  [sizeof=4] blink::InlineBox::InlineBoxBitfields bitfields_
  <padding> (4 bytes)
  [sizeof=8] blink::InlineBox* next_
  [sizeof=8] blink::InlineBox* prev_
  [sizeof=8] blink::InlineFlowBox* parent_
  [sizeof=8] blink::LineLayoutItem line_layout_item_
  [sizeof=8] blink::LayoutPoint location_
  [sizeof=4] blink::LayoutUnit logical_width_
  <padding> (4 bytes)
}

class layout after the change:

class blink::InlineBox [sizeof = 64]
  : public blink::DisplayItemClient {
  <padding> (4 bytes)
  [sizeof=8] blink::InlineBox* next_
  [sizeof=8] blink::InlineBox* prev_
  [sizeof=8] blink::InlineFlowBox* parent_
  [sizeof=8] blink::LineLayoutItem line_layout_item_
  [sizeof=8] blink::LayoutPoint location_
  [sizeof=4] blink::LayoutUnit logical_width_
  [sizeof=4] blink::InlineBox::InlineBoxBitfields bitfields_
}

BUG=710933

Review-Url: https://codereview.chromium.org/2829683010
Cr-Commit-Position: refs/heads/master@{#466699}
Committed:
https://chromium.googlesource.com/chromium/src/+/8a9eb48c6d24472e6812811ece2b...
==========

[commit-bot: I haz the power, 2017-04-24 18:51:42]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/8a9eb48c6d24472e6812811ece2b...

[brucedawson, 2017-04-24 19:26:12]

On 2017/04/24 18:51:42, commit-bot: I haz the power wrote:
> Committed patchset #1 (id:1) as
>
https://chromium.googlesource.com/chromium/src/+/8a9eb48c6d24472e6812811ece2b...

Technically it "eliminates most padding", rather than "eliminates padding", but
great work! I'm very pleased with these results.