Demigiant / dotween

A Unity C# animation engine. HOTween v2
http://dotween.demigiant.com
Other
2.34k stars 351 forks source link

Best action on killing all tweens on transform #151

Open kevinkassimo opened 7 years ago

kevinkassimo commented 7 years ago

Hi, this might be more of a problem than an issue.

As I am working on a game involving thousands of bullets moving on screen, I need to recycle bullets on hitting the screen boundary. As I am doing so, I currently have to call transform.DOKill() on each bullet transform. However, I found serious performance drops when I do this way, as shown in Profiler, DOKill takes more than 40% (UPDATE: actually 60%, after rechecking) of the CPU time. It seems that killing the tweens takes much more CPU time than normal tweening.

So what can be a better way than directly calling transform.DOKill()? In my case, the exact time of bullet recycling is very unpredictable. Thanks!

kevinkassimo commented 7 years ago

(Also, a more detailed review of profiler report shows more than 35% of the time spent on DOTween.Kill() is some Object.Equals() inside of TweenManager.FilteredOperation())

I just checked the source code of FilteredOperation(). It seems that killing all tweens on a transform requires looping through all _activeTweens and falls into case FilterType.TargetOrId: of filterType, in which *.Equals() shows up. Would there be a better way to avoid this by somehow caching the references of the tweens directly on some list entry dedicated to a certain object? (I did not view the complete source code on tweens and I am not a professional coder, so I may be wrong on this...)

kevinkassimo commented 7 years ago

After reviewing everything again and comparing to the source code of LeanTween (LeanTween.cancel(), from here ), I found both looping through all tweens when killing tweens on Transform/GameObject. However, LeanTween also offers a way of killing using ID, in which I found no loops (which will be fine for me, as I can cache the ID myself in each bullet script). However, for the DOTween case, it seems the ID and Transform both goes with the same huge loop on all _activeTweens. I really enjoy DOTween with its highly intuitive API design, but this performance issue is almost killing any further development of my project, so I would hope if there could be a solution to it. Thanks!

Demigiant commented 7 years ago

Hi,

This issue arose recently. That happens because DOTween's ID allows to use any object type as an ID, and thus requires an equals to verify if it's correct. I'm planning to add an IntID, so that filtering that way can be much faster. I hope to find some time to work on it this weekend.

Cheers, Daniele

Demigiant commented 7 years ago

Hey,

Wanna try this new version? SetId has the same API, but if you pass a string or int ID it stores the ID as string/int instead of objects, so equality comparison is much faster (using a string is 2X faster than before, int is 4X faster).

Transform.DOKill will still be the same, since it necessarily needs to check for object equality (60% seems like a lot though-unless the overall MS time for that frame is very low and nothing much is happening-maybe there's something happening when you kill those tweens, which is thus put under the kill operation by the Profiler, but is independent from it?), but if you set an int ID and use DOTween.Kill(intId) it will be much much faster.

Let me know, and cheers

kevinkassimo commented 7 years ago

Great! I will test out this new feature as soon as I go back to work on my project tomorrow. Thanks!

Just as an extra thought: reading the huge loop inside FilteredOperation() again, I did not find other use case of the index i other than fetching _activeTweens[i]. Does it mean that if I just keep a reference of the target Tween and call some slightly modified version (e.g. Adding lines that check t.active first and deals with the _KillList etc. as seen in the loop and the block run on hasDespawned being true) of Despawn() would also do the Kill trick? I may be missing some point since I found pooling and some other design structures in TweenManager.cs (I review the code currently on mobile device so I may be not careful enough), but I would like to know if my instinct is correct and can avoid the huge loop after all.

Demigiant commented 7 years ago

I'm not sure of what you mean :P If you have the tween reference, then you can directly call all methods on it, like myTween.Kill/Play/Pause/etc, and it will skip the FilteredOperation loop. Did I misunderstand?

kevinkassimo commented 7 years ago

Got it. It was me that misunderstood the problem... Thanks for the hint!

kruncher commented 6 years ago

This sounds very interesting. I am myself finding that DOTween.Kill is causing a noticable stutter when running on the iPhone; and we're not really talking about that many game objects.

Each time an object is pooled I am calling the kill function on two objects (for separation purposes it's necessary to maintain tweens on two objects):

DOTween.Kill(this.Context.GameObject);
DOTween.Kill(this);

At most there are only 10 of these objects on the screen at any given time. And this operation occurs each time the screen is tapped. With each tap the entire screen pauses for a brief moment and the culrit seems to be this usage of DOTween.Kill since if I remove those above calls the stutter vanishes.

Debug.Log(DOTween.TotalPlayingTweens()); is also reporting that there are not a significant number of tweens active at any given time; between 0 and 20.

kruncher commented 6 years ago

Btw just to add; I'm seeing 20-30% CPU usage coming from this.

kruncher commented 6 years ago

On initial inspection I can see in the experimental download above that the integer value would be boxed each time the FilteredOperation method is called leading to garbage/collection.

kruncher commented 6 years ago

Out of curiosity; what if these kill operations were to be queued and then processed in bulk? This way only looping over the collection of tweens one time. To improve lookup performance why not use a dictionary or use .GetHashCode on the id?

aDu commented 3 years ago

Did you find a good way to solve this @kruncher or @kevinkassimo?

Edit: The comment above seems like a good way. If it's not a popular feature on demand, I guess I'll just workaround it.

Edit: I don't need it for my use-case anymore.

shroudpatric commented 3 years ago

I find an easy way but not suitable for everybody. If you're using the pro version of DoTween,you can avoid the above question about Object.Equals() called many times by doing this: Using DoTweenAnimation.tween.Kill() instead of DoTweenAnimation.DoKill(); Using DoTweenAnimation.tween.Complete() instead of DoTweenAnimation.DoComplete();

builder-main commented 1 year ago

Effectivelly calling kill on the object instead of the tween itself is really killing (haha) performances. I believe keeping the reference of the tween instead of calling the object extension method DOKill would benefit a lot. Is this what you're saying @Demigiant ?

Demigiant commented 1 year ago

@builder-main Ahoy! Absolutely! Killing based on an object reference comparison is the most expensive way. Keeping a tween reference and killing it directly is the best way with no overhead. Other than that, killing tweens by id also has very little overhead especially if you assigned an int id instead of a string (because I process them differently for speed). And finally, KillAll also has no overhead

builder-main commented 1 year ago

I did this and it removed all overhead that was caused by DOKill. So it's great ! I had some issue with ids in the past, so I'm more confident with keeping the references of the tweens plus it gives the finest control over lifetime, KillAll also won't do in our case as we have tweening all over the place :) Thanks !

Demigiant commented 1 year ago

Yay! Personally I do the same and always kill by direct tween references, though sometimes I use int ids for UI tweens so I can quickly kill all tweens except the UI ones (in case of loaders that switch to a new scene) via the KillAll overload that allows to exclude tweens with a given id :)

builder-main commented 1 year ago

Yay! Personally I do the same and always kill by direct tween references, though sometimes I use int ids for UI tweens so I can quickly kill all tweens except the UI ones (in case of loaders that switch to a new scene) via the KillAll overload that allows to exclude tweens with a given id :)

That looks like an interesting usecase indeed.