Disable g++ inlining of eager-tracing mark() method.

Disable g++ inlining of eager-tracing mark() method.

Versions of g++ (with -Os) are over-eager about inlining the mark()
method that's used for all non-mixin Oilpan objects, resulting in
a code size increase that's unwanted (for Android official builds.)
Other compilers and g++ (with -O2/-O3) are more discriminate about
inlining the method, with no comparable code size increase delta.

Tuning the compiler's optimization option set to avoid the problem
hasn't proven successful, so bluntly address the problem by disabling
inlining for the method.

R=haraken
BUG=681991
Review-Url: https://codereview.chromium.org/2643403003
Cr-Commit-Position: refs/heads/master@{#445288}
Committed: https://chromium.googlesource.com/chromium/src/+/9fa52ef0da5a0a6e03a15c4c72d8021567070127

[sof, 2017-01-21 07:41:19]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-21 07:41:31]

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

[sof, 2017-01-21 08:48:13]

sigbjornf@opera.com changed reviewers:
+ agrieve@chromium.org, oilpan-reviews@chromium.org

[sof, 2017-01-21 08:48:14]

please take a look.

I observe a 360k reduction in size for libcontent_shell_content_view.so with
this change for a target_os=android official build.

[commit-bot: I haz the power, 2017-01-21 09:17:52]

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

[commit-bot: I haz the power, 2017-01-21 09:17:53]

Dry run: This issue passed the CQ dry run.

[haraken, 2017-01-21 13:15:36]

LGTM.

Just to confirm: Is this a specific problem in g++ builds (i.e., not a problem
in clang builds)?

[sof, 2017-01-21 13:40:28]

On 2017/01/21 13:15:36, haraken wrote:
> LGTM.
> 
> Just to confirm: Is this a specific problem in g++ builds (i.e., not a problem
> in clang builds)?

That's right, arm & g++ specific. Confirmed not to reproduce with clang. For
x86, no problem with either toolchain.

Given that android uses the problematic toolchain for official builds, I think
we should try this, but marking slowdown is the risk.

[sof, 2017-01-21 14:18:02]

Description was changed from

==========
Disable g++ inlining of eager-tracing mark() method.

Versions of g++ (with -Os) are over-eager about inlining the mark()
method that's used for all non-mixin Oilpan objects, resulting in
a code size increase that's unwanted (for Android official builds.)
Other compilers and g++ (with -O2/-O3) are more discriminate about
inlining the method, with no comparable code size increase delta.

Tuning the compiler's optimization option set to avoid the problem
hasn't proven successful, so bluntly address the problem by disabling
inlining for the method.

R=
BUG=681991
==========

to

==========
Disable g++ inlining of eager-tracing mark() method.

Versions of g++ (with -Os) are over-eager about inlining the mark()
method that's used for all non-mixin Oilpan objects, resulting in
a code size increase that's unwanted (for Android official builds.)
Other compilers and g++ (with -O2/-O3) are more discriminate about
inlining the method, with no comparable code size increase delta.

Tuning the compiler's optimization option set to avoid the problem
hasn't proven successful, so bluntly address the problem by disabling
inlining for the method.

R=haraken
BUG=681991
==========

[sof, 2017-01-21 14:18:29]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2017-01-21 14:18:40]

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-01-21 14:26:41]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1485008309612200, "parent_rev":
"858ceafc7cf4f11a6549b8c1ace839a45d943d68", "commit_rev":
"9fa52ef0da5a0a6e03a15c4c72d8021567070127"}

[commit-bot: I haz the power, 2017-01-21 14:27:17]

Description was changed from

==========
Disable g++ inlining of eager-tracing mark() method.

Versions of g++ (with -Os) are over-eager about inlining the mark()
method that's used for all non-mixin Oilpan objects, resulting in
a code size increase that's unwanted (for Android official builds.)
Other compilers and g++ (with -O2/-O3) are more discriminate about
inlining the method, with no comparable code size increase delta.

Tuning the compiler's optimization option set to avoid the problem
hasn't proven successful, so bluntly address the problem by disabling
inlining for the method.

R=haraken
BUG=681991
==========

to

==========
Disable g++ inlining of eager-tracing mark() method.

Versions of g++ (with -Os) are over-eager about inlining the mark()
method that's used for all non-mixin Oilpan objects, resulting in
a code size increase that's unwanted (for Android official builds.)
Other compilers and g++ (with -O2/-O3) are more discriminate about
inlining the method, with no comparable code size increase delta.

Tuning the compiler's optimization option set to avoid the problem
hasn't proven successful, so bluntly address the problem by disabling
inlining for the method.

R=haraken
BUG=681991

Review-Url: https://codereview.chromium.org/2643403003
Cr-Commit-Position: refs/heads/master@{#445288}
Committed:
https://chromium.googlesource.com/chromium/src/+/9fa52ef0da5a0a6e03a15c4c72d8...
==========

[commit-bot: I haz the power, 2017-01-21 14:27:18]

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

[sof, 2017-01-23 13:24:17]

On 2017/01/21 08:48:14, sof wrote:
> please take a look.
> 
> I observe a 360k reduction in size for libcontent_shell_content_view.so with
> this change for a target_os=android official build.

Re: 360k, this was with dcheck_always_on=true.

[agrieve, 2017-01-23 16:00:32]

On 2017/01/23 13:24:17, sof wrote:
> On 2017/01/21 08:48:14, sof wrote:
> > please take a look.
> > 
> > I observe a 360k reduction in size for libcontent_shell_content_view.so with
> > this change for a target_os=android official build.
> 
> Re: 360k, this was with dcheck_always_on=true.

The bots report 140kb saved by this:
https://chromeperf.appspot.com/report?sid=cfc29eed1238fd38fb5e6cf83bdba6c619b...

Could be the difference is from dcheck_always_on. Still, great work!

[sof, 2017-01-23 16:14:40]

On 2017/01/23 16:00:32, agrieve wrote:
> On 2017/01/23 13:24:17, sof wrote:
> > On 2017/01/21 08:48:14, sof wrote:
> > > please take a look.
> > > 
> > > I observe a 360k reduction in size for libcontent_shell_content_view.so
with
> > > this change for a target_os=android official build.
> > 
> > Re: 360k, this was with dcheck_always_on=true.
> 
> The bots report 140kb saved by this:
>
https://chromeperf.appspot.com/report?sid=cfc29eed1238fd38fb5e6cf83bdba6c619b...
> 
> Could be the difference is from dcheck_always_on. Still, great work!

Thanks, I suspect it is - there's a pair of DCHECK()s in that method, which were
inlined also. That "g++ -Os" still wanted to inline, is even more unusual.