Force more inlining in Skia ARM NEON optimizations

Force more inlining in Skia ARM NEON optimizations

The Skia ARM NEON optimizations suffer from lack of inlining somewhat.
Force more agressive optimization and inline bigger functions for the
skia_opts_neon target.

BUG=363073

[Kimmo Kinnunen, 2014-04-14 12:28:47]

or any idea where would the 'variables': { 'optimize': 'max' } or similar thing
go ?

[tomhudson, 2014-04-14 12:30:40]

Nice catch! I'll leave it to the others to figure out gyp layout & provide
formal review.

[Kimmo Kinnunen, 2014-05-07 12:11:38]

Ping? Anybody know anybody willing to look at the patch?

[tomhudson, 2014-05-07 13:10:53]

Not sure why we're listing Ben? Added Leon & Mike as potential ARM reviewers.
Fired off some trybots.

Have you measured code size impact, or quantified the performance improvements?
We're looking at similar changes at higher levels of the stack right now on
Android, but the time/space tradeoff is a tight one.

[mtklein, 2014-05-07 14:49:42]

https://codereview.chromium.org/225853003/diff/1/skia/skia_library_opts.gyp
File skia/skia_library_opts.gyp (right):

https://codereview.chromium.org/225853003/diff/1/skia/skia_library_opts.gyp#n...
skia/skia_library_opts.gyp:248: # The special cases are performance critical and
suffer from lack
Is there any way we can do this with SK_ALWAYS_INLINE (==
__attribute__((always_inline)) ) tagged on particular functions in source?  My
concerns about doing in the build are:
  1) Next time we come to fiddle with this, we won't know which functions this
was meant to help.
  2) We're only doing this in Chrome, so Android loses out, and we're not
testing it on our Skia bots.

[Kimmo Kinnunen, 2014-05-13 12:12:53]

On 2014/05/07 13:10:53, tomhudson wrote:
> Have you measured code size impact, or quantified the performance
improvements?
> We're looking at similar changes at higher levels of the stack right now on
> Android, but the time/space tradeoff is a tight one.

The library size does not change at all, eg.
out/Release/obj/skia/libskia_opts_neon.a is 8108 bytes before and after the
change. I tried to inspect the other libraries too, but I did not manage to see
any difference.

The perf improvement is quite significant on NV hw today, but it's probably due
to us hitting a degenerate case in our hw/sw with the un-inlined case. The
badness will probably be fixed, so the perf improvement in the end should be
small(ish), in this case.

I'm not sure the implications. One could consider inspecting that functions
using neon are forced to be inlined. They seem to be somewhat big, and thus
might remain uninlined otherwise. Of course, such an inspection is quite a task
for project like Chromium or Android.


On 2014/05/07 14:49:42, mtklein wrote:
> https://codereview.chromium.org/225853003/diff/1/skia/skia_library_opts.gyp
> File skia/skia_library_opts.gyp (right):
> 
>
https://codereview.chromium.org/225853003/diff/1/skia/skia_library_opts.gyp#n...
> skia/skia_library_opts.gyp:248: # The special cases are performance critical
and
> suffer from lack
> Is there any way we can do this with SK_ALWAYS_INLINE (==
> __attribute__((always_inline)) ) tagged on particular functions in source?  My
> concerns about doing in the build are:
>   1) Next time we come to fiddle with this, we won't know which functions this
> was meant to help.
>   2) We're only doing this in Chrome, so Android loses out, and we're not
> testing it on our Skia bots.

That's a good idea, thanks. I submitted a patch to Skia,
https://codereview.chromium.org/280403005/ . Closing this one.