New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 711101 link

Starred by 1 user
Status: Fixed
Owner:
bashi@chromium.org
Closed: Apr 2017
Cc:
█ zturner@chromium.org
bashi@chromium.org
lfg@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocking:
issue 710933



Sign in to add a comment

dictionary_impl.h.tmpl causes inefficient structures such as ConstrainDoubleRange

Project Member Reported by brucedaw...@chromium.org, Apr 13 2017 
Some recent analysis by zturner@ pointed out that dictionary_impl.h.tmpl is generating classes with significant amounts of internal padding. The alternating of bool flags with payload members leads to up to seven bytes of padding after each bool (if the payload is a double).

In particular, consider ConstrainDoubleRange which inherits from DoubleRange and therefore has four bool/double pairs, each with seven bytes of padding. The net result is that this class has 36 bytes of data and 28 bytes of padding. Putting the bool flags adjacent to each other within each class would give us two blocks padding each with six bytes, for a total of twelve bytes, a sixteen-byte savings.

Greater savings could be realized if the bool variables could be eliminated, perhaps by using special values (NaN, infinity, -DBL_MAX) to indicate missing values, although this may not be practical.

It looks like this behavior was introduced in crrev.com/2494973002. I don't know how many classes or objects this affects.

The seven bytes of padding after each bool is true on 32-bit and 64-bit Windows builds. It may be true on other platforms or the padding may be less, especially on 32-bit.

The recommended minimum fix is to change the code generator so that the bool flags are always adjacent. This should produce results that are better in many cases with no downsides.

Comment 1 by brucedaw...@chromium.org, Apr 13 2017

Cc: bashi@chromium.org
CCing the author of crrev.com/2494973002

Comment 2 by bashi@chromium.org, Apr 14 2017

Components: Blink>Bindings
Owner: bashi@chromium.org
Status: Assigned (was: Untriaged)
Thanks for the report!

Comment 4 by brucedaw...@chromium.org, Apr 14 2017 

Do you know how many types and how many objects this will affect? ConstrainDoubleRange and DoubleRange (28 bytes and 14 bytes of padding each, should drop to 12 and 6 respectively) are the only ones I'm aware of but even this those I don't have any sense of how many of them there are.

Thanks for the quick fix!

Comment 5 by brucedaw...@chromium.org, Apr 15 2017

Blocking: 710933

Comment 6 by bashi@chromium.org, Apr 17 2017

Status: Fixed (was: Assigned)
I have no idea about how many types/objects are affected, too :(

Let me close this. There is a room for improvement (e.g. using bit flags) but I'd prefer this simple solution.

Comment 7 by jbroman@chromium.org, Apr 17 2017 

I'd be surprised if we had a lot of these objects; they're mostly used for passing parameters to/from JavaScript, so I'd expect them to be short-lived. Still, doesn't hurt to remove the padding.

Comment 8 by brucedaw...@chromium.org, Apr 17 2017 

Sounds good. For the specific case we were looking at I think that the only way to get further improvement would be to use special sentinel double values to indicate that no number is available (a specific NaN pattern, infinity, etc.) but that comes with tradeoffs and complications that aren't justified unless the memory savings are significant, and it doesn't sound like they are.

Closing is good.
►

Sign in to add a comment