TouchProcessor.enqueue is creating too many arrays

[hardcoremore]

https://github.com/Gamua/Starling-Framework/blob/365ece4199385d2e197219a2fa57114b42cb818d/starling/src/starling/events/TouchProcessor.as#L229

Can this array be pooled instead of created every time?

This method is creating too many Array instances. Take a look at the screenshot: https://ibb.co/bSM0Zn

And here is the Scout Profile: https://goo.gl/NYXced

This was tested on iPhone 6s.

[gsario]

@hardcoremore good catch! @PrimaryFeather any ideas? Can this be also related to Android ANR-s related to touch?

[hardcoremore]

Now that I think about it, I think it is a very easy solution to this. It is a little more lines of code but far less allocations. I think this can be safely replaced with this:

_queue.unshift(_touchMarker.mockY);
_queue.unshift(_touchMarker.mockX);
_queue.unshift(phase);
_queue.unshift(1);

What do you guys think?

Other thing that bothers me now is that I have read the comment above that says:

// multitouch simulation (only with mouse)

but I have tested it with iPhone 6s and Adobe Scout has reported allocations in that function?

But how is that possible since this should be used only to simulate multi touch and only when ctrl is pressed down?

[JohnBlackburne]

That code only runs when using multitouch simulation, so it should not show up when profiling a production build, either on mobile or desktop.

[hardcoremore]

Yeah, it is strange. Well I need to test that again with Scout to see if it is really running when multi touch simulation is not used.

[PrimaryFeather]

Sorry for the late reply, I somehow overlooked this post!

In any case, that code only runs for multitouch simulation, so it shouldn't be critical. Furthermore, you cannot replace it with multiple unshift calls, because the queue needs to be filled with Arrays of arguments, not the arguments themselves.

And even if we could, it would have the contrary effect: since unshift uses variable arguments (... args), it will create one array per call (!) on iOS. (This is the same reason why Array.push() is only used very rarely in Starling.)

In other words, I'd rather leave this as it is. :wink:

[hardcoremore]

Hi Daniel,

Thanks for the replay. Can you just please give a little more details about:

"it will create one array per call (!) on iOS. (This is the same reason why Array.push() is only used very rarely in Starling."

Is that a new bug for iOS I am not aware? Why is Array.push() avoided in Starling?

Thanks

[PrimaryFeather]

"it will create one array per call (!) on iOS. (This is the same reason why Array.push() is only used very rarely in Starling.)" Is that a new bug for iOS I am not aware? Why is Array.push() avoided in Starling?

No, this is not new; it's just something the AOT compiler does not optimize (or cannot, for whatever reason). Whenever you've got a method with a variable number of arguments, an array will be created and filled with the parameters you passed into the method. The JIT-compiler seems to be able to optimize this, but the AOT compiler can't.

Array.push() is another method with variable arguments, so it's better avoided, too. Array.insertAt() can be used instead, though — that was added on Josh's and my recommendation as a workaround.

[hardcoremore]

Oh so whenever I use Array.push or Vector.push an array will be allocated regardless if I just pass a object as single parameter?

[PrimaryFeather]

Exactly! On iOS, at least.

[hardcoremore]

Ok great thanks. I already started to refactor to eliminate all push. Sigh, so many little different things for each platform :)

[PrimaryFeather]

Yeah, I can feel with you. :wink: Very annoying indeed!

[hardcoremore]

I can confirm 100% that TouchProcessor.enqueue is creating too much arrays.

This is because of using unshift method. It is affecting Android and iOS as well.

Take a look at the Adobe Scout profile screenshot:

https://ibb.co/bTws6T

I have refactored this to not use unshift method and arguments parameter. Instead I created new class TouchData that is pooled and not I have a lot less allocations.

[JohnBlackburne]

It looks like you are profiling a desktop version again, not Android or iOS. Apart from unshift only being called if _ctrlDown is set, which can only happen on desktop, your screenshot says Scout recorded processing of a mouseMove event which can only happen on desktop.

[hardcoremore]

Hi John,

No I am not profiling Desktop version. Those allocations are happening not just because of using unshift but because using arguments parameter as well.

I have refactored _queue variable to not be a Vector.<Array> but Vector.<TouchData> instead where TouchData class is pooled.

[PrimaryFeather]

Hm, yeah, it could be that using arguments is another way to force an array creation. I'll look into it!

[hardcoremore]

Thanks Daniel.

This is my version of TouchProcessor class that I am using now. I also implemented pooling of Touch objects as well:

https://pastebin.com/RfAXb1MG

And here is the TouchData class:

https://pastebin.com/9xdXfbzv

Thanks

[PrimaryFeather]

Thanks for sharing the code!

One problem that I noticed is that this change will break code of developers who extended the TouchProcessor class, since the _queue member is protected. This will probably not have been done very often, but still ... the question remains if this optimization is worth breaking existing code.

[hardcoremore]

Hi Daniel,

Yes, unfortunately it breaks backwards compatibility. I don't know if that change is worth for general uses but it is definitely worth for me. I am now running Starling from my branch.

My guess is that it is probably worth it for general use case as well because it prevents allocations which is always good and change is not that big and probably very little percent of people are extending that class anyway.

[PrimaryFeather]

I tend to agree, but I'll take another look! Cheers!

[PrimaryFeather]

I finally committed this! I agree that the compatibility issue is a price worth paying in this case.

[hardcoremore]

Awesome, thanks!