Closed Bug 1219352 Opened 4 years ago Closed 4 years ago

Clean up fields in FrameMetrics

Categories

(Core :: Graphics: Layers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox45 --- wontfix
firefox46 --- fixed

People

(Reporter: kats, Assigned: kats)

Details

(Whiteboard: [gfx-noted])

Attachments

(3 files)

The FrameMetrics class has grown over time and gotten a bit messy. In particular there are a number of bool fields which could probably be packed together better. We should do that since we use FrameMetrics quite a bit and it's worth spending a bit of time to make sure it's not taking up unnecessary amounts of space.
Whiteboard: [gfx-noted]
Assignee: nobody → bugmail.mozilla
This patch:
- Maintains a consistent ordering between the lists of fields in the
  FrameMetrics constructor, operator==, IPC read/write functions, and the
  actual order of fields in FrameMetrics. As part of this, missing default
  initializers are added to the FrameMetrics constructor, and fields omitted
  from the operator== are explicitly noted.
- Moves all the boolean values to the end of the set of field (for better
  packing).
- Moves the scroll id and parent scroll id to the front of the list, so that
  the operator== can fail faster in the common case.

Review commit: https://reviewboard.mozilla.org/r/29607/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/29607/
Attachment #8704276 - Flags: review?(botond)
Attachment #8704275 - Flags: review?(botond) → review+
Comment on attachment 8704275 [details]
MozReview Request: Bug 1219352 - Update a couple of bool-setters to take a bool argument. r?botond

https://reviewboard.mozilla.org/r/29605/#review26573
Attachment #8704276 - Flags: review?(botond) → review+
Comment on attachment 8704276 [details]
MozReview Request: Bug 1219352 - Rearrange fields in FrameMetrics. r?botond

https://reviewboard.mozilla.org/r/29607/#review26579
Comment on attachment 8704277 [details]
MozReview Request: Bug 1219352 - Pack the FrameMetrics booleans. r?botond

https://reviewboard.mozilla.org/r/29609/#review26581

It looks like over the wire, the bools still take up sizeof(int) bytes. Assuming the intention of this patch wasn't to optimize that, this is fine.
Attachment #8704277 - Flags: review?(botond) → review+
(In reply to Botond Ballo [:botond] from comment #7)
> It looks like over the wire, the bools still take up sizeof(int) bytes.
> Assuming the intention of this patch wasn't to optimize that, this is fine.

Good point - yes, the intention wasn't to optimize that since that's really transient memory and optimizing it is probably not worth it.

For the record on OS X FrameMetrics was 304 bytes to start with and went down to 272 after the changes, if I'm remembering right. It was in the neighbourhood of a 10% improvement at any rate.
You need to log in before you can comment on or make changes to this bug.