Inline gfx::QuadF and gfx::Transform ctor/dtors

Inline gfx::QuadF and gfx::Transform ctor/dtors

This speeds up cc_perftests by 2.3% compared to ToT and 3% after other optimizations land.

BUG=


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=174902

[jamesr, 2012-12-02 02:26:55]

Hey Elliot - this patch speeds up a benchmark I care a lot about by 3%, but the
clang FindBadConstructs plugin dislikes it because (according to it) QuadF has 4
non trivial members.  The definition of QuadF is:

class QuadF {
 private:
  PointF p1_;
  PointF p2_;
  PointF p3_;
  PointF p4_;
};

and PointF is derived from a templated base class but is actually just a pair of
floats, which means QuadF is really a set of 8 floats.  PointF's ctors/dtors are
all inline, so the default ctor for QuadF sets 8 floats to 0.0 and the dtor for
QuadF generates no code at all.

How can I land this?  I could bump FindBadConstruct's non_trivial_member
multiplier from 3 to 2, try to refine the non_trivial_member calculation so it
recognizes that PointF really is trivial, or add some sort of
annotation/blacklist/whitelist mechanism.

[danakj, 2012-12-03 16:49:07]

LGTM if you can make clang happy

[Elliot Glaysher, 2012-12-03 18:54:59]

On 2012/12/02 02:26:55, jamesr wrote:
> Hey Elliot - this patch speeds up a benchmark I care a lot about by 3%, but
the
> clang FindBadConstructs plugin dislikes it because (according to it) QuadF has
4
> non trivial members.  The definition of QuadF is:
> 
> class QuadF {
>  private:
>   PointF p1_;
>   PointF p2_;
>   PointF p3_;
>   PointF p4_;
> };
> 
> and PointF is derived from a templated base class but is actually just a pair
of
> floats, which means QuadF is really a set of 8 floats.  PointF's ctors/dtors
are
> all inline, so the default ctor for QuadF sets 8 floats to 0.0 and the dtor
for
> QuadF generates no code at all.
> 
> How can I land this?  I could bump FindBadConstruct's non_trivial_member
> multiplier from 3 to 2, try to refine the non_trivial_member calculation so it
> recognizes that PointF really is trivial, or add some sort of
> annotation/blacklist/whitelist mechanism.

Add it to the whitelisted classes in tools/clang/plugins/ChromeClassTester.cpp.
Add a comment that this actually has a perf progression.

[jamesr, 2012-12-03 19:08:41]

On 2012/12/03 18:54:59, Elliot Glaysher wrote:
> Add it to the whitelisted classes in
tools/clang/plugins/ChromeClassTester.cpp.
> Add a comment that this actually has a perf progression.

https://codereview.chromium.org/11280279/

I'll need that to land and a clang roll to happen before landing, right?

[Elliot Glaysher, 2012-12-03 19:14:37]

+thakis

I thought that we could just land changes to the plugin in affected patches.

[jamesr, 2012-12-04 05:46:30]

This can land once the clang plugin rolls past r170868.

[commit-bot: I haz the power, 2013-01-02 22:59:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/11299290/7001

[commit-bot: I haz the power, 2013-01-03 00:49:49]

Change committed as 174902