Avoid heap allocation with uncoalesced touch events

Avoid heap allocation with uncoalesced touch events

Each enqueued event in the browser-side touch queue maintains a vector
of events that must be ack'ed, allowing each enqueued event to coalesce
with subsequent events delivered from the platform. Currently, the
vector is always populated with the first event, even if no coalescing
occurs. Instead, defer population until coalescing occurs, optimizing
the common case for most events by avoiding an extra heap allocation for
every touch. This can save ~5-10us per forwarded touch event on a typical
Android device.

BUG=384562
Committed: https://crrev.com/59886ae9da6fc726873723cc2ea9d0db10adab22
Cr-Commit-Position: refs/heads/master@{#295998}

[jdduke (slow), 2014-09-11 23:53:05]

jdduke@chromium.org changed reviewers:
+ miletus@chromium.org

[jdduke (slow), 2014-09-11 23:53:06]

miletus@: PTAL, thanks.  Another thought would be to have a ring buffer of
CoalescedTouchEvents that get recycled, but this was pretty low hanging fruit.

[Yufeng Shen (Slow to review), 2014-09-12 19:11:55]

On 2014/09/11 23:53:06, jdduke wrote:
> miletus@: PTAL, thanks.  Another thought would be to have a ring buffer of
> CoalescedTouchEvents that get recycled, but this was pretty low hanging fruit.

does StackVector meet your needs ?

[jdduke (slow), 2014-09-12 19:27:32]

On 2014/09/12 19:11:55, Yufeng Shen wrote:
> On 2014/09/11 23:53:06, jdduke wrote:
> > miletus@: PTAL, thanks.  Another thought would be to have a ring buffer of
> > CoalescedTouchEvents that get recycled, but this was pretty low hanging
fruit.
> 
> does StackVector meet your needs ?

I suppose we could just use a StackVector of size 1?

[Yufeng Shen (Slow to review), 2014-09-12 19:40:04]

On 2014/09/12 19:27:32, jdduke wrote:
> On 2014/09/12 19:11:55, Yufeng Shen wrote:
> > On 2014/09/11 23:53:06, jdduke wrote:
> > > miletus@: PTAL, thanks.  Another thought would be to have a ring buffer of
> > > CoalescedTouchEvents that get recycled, but this was pretty low hanging
> fruit.
> > 
> > does StackVector meet your needs ?
> 
> I suppose we could just use a StackVector of size 1?

Yeah, using StackVector with size 1 would keep the original code flow; while
your 
approach can avoid an unnecessary copy when there is no coalescing. I don't have
strong preference.

[jdduke (slow), 2014-09-15 16:42:10]

On 2014/09/12 19:40:04, Yufeng Shen wrote:
> Yeah, using StackVector with size 1 would keep the original code flow; while
> your 
> approach can avoid an unnecessary copy when there is no coalescing. I don't
have
> strong preference.

I ran the perf tests with a StackVector and got performance somewhere between
the two approaches (using content_perftests). Not sure if the marginal gain is
worth the change in that case, so maybe I'll just sit on this patch for now...

I actually have a patch that uses scoped_ptr's everywhere, preventing all of the
intermediate copying we need for pushing and for modifying LatencyInfos. I don't
think it was a big win, but I might try revisiting that.

[Yufeng Shen (Slow to review), 2014-09-15 18:58:33]

On 2014/09/15 16:42:10, jdduke wrote:
> On 2014/09/12 19:40:04, Yufeng Shen wrote:
> > Yeah, using StackVector with size 1 would keep the original code flow; while
> > your 
> > approach can avoid an unnecessary copy when there is no coalescing. I don't
> have
> > strong preference.
> 
> I ran the perf tests with a StackVector and got performance somewhere between
> the two approaches (using content_perftests). Not sure if the marginal gain is
> worth the change in that case, so maybe I'll just sit on this patch for now...
> 
> I actually have a patch that uses scoped_ptr's everywhere, preventing all of
the
> intermediate copying we need for pushing and for modifying LatencyInfos. I
don't
> think it was a big win, but I might try revisiting that.

lgtm.

[jdduke (slow), 2014-09-22 16:02:10]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-09-22 16:03:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/565033002/20001

[commit-bot: I haz the power, 2014-09-22 16:53:37]

Committed patchset #2 (id:20001) as 4507604e238fca14c3552e5cd7da15f8c19baa7c

[commit-bot: I haz the power, 2014-09-22 16:54:14]

Patchset 2 (id:??) landed as
https://crrev.com/59886ae9da6fc726873723cc2ea9d0db10adab22
Cr-Commit-Position: refs/heads/master@{#295998}