search

truyet / hotween

Automatically exported from code.google.com/p/hotween
0 stars 1 forks source link

SVN Patch (test) #36

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Hey, just a test if patch will merge fine.
It's a tiny start of low level optimizations (hope, I'll make some more).

Original issue reported on code.google.com by dmitry....@gmail.com on 9 Aug 2012 at 1:30

Attachments:

GoogleCodeExporter commented 9 years ago 
Ouch, those you optimized were almost bugs, considering my stupid repetition of 
stuff. Thanks for that :) If you want to contribute more I'll be very happy, 
and will give you commit permission after the next patch.

Seeing that I'm at it, I'm gonna optimize HOTween's update iteration now.

Original comment by daniele....@gmail.com on 9 Aug 2012 at 8:43

GoogleCodeExporter commented 9 years ago 
Great, I'll wait for the next release and see if I can help with optimization 
further then.

Original comment by dmitry....@gmail.com on 9 Aug 2012 at 10:13

GoogleCodeExporter commented 9 years ago 
I scrapped my optimization. I optimized the update iteration and then realized 
the optimization was broken (way less code, but less efficient :P).

Original comment by daniele....@gmail.com on 9 Aug 2012 at 3:50

GoogleCodeExporter commented 9 years ago 
I hope you're not extremely drunk after that donation, haha!)
So are you still working on Update optimizations?

Original comment by dmitry....@gmail.com on 9 Aug 2012 at 3:54

GoogleCodeExporter commented 9 years ago 
Not yet drunk, but I have a BIG bottle of beer next to me :D
I don't think I'll work on optimizations for the rest of the week. Left my game 
behind and will probably concentrate on that until Sunday.

Original comment by daniele....@gmail.com on 9 Aug 2012 at 5:23

GoogleCodeExporter commented 9 years ago 
Haha, I see, maybe I'll provide one more patch in a few days with additional 
low-level optimizations then)

Original comment by dmitry....@gmail.com on 9 Aug 2012 at 5:25

GoogleCodeExporter commented 9 years ago 
Wonderful :) If I should change my mind and start touching things around (or if 
some bug appears and I need to fix it) I'll warn you.

Original comment by daniele....@gmail.com on 9 Aug 2012 at 5:29

GoogleCodeExporter commented 9 years ago 
I got a cool suggestion for an addition to callbacks, in order to let them 
implement Unity sendMessages (which I don't like at all, but would work nice 
with the Visual Editor). So I'm starting to touch stuff, but nothing that 
should cause issues in case of a merge.

Original comment by daniele....@gmail.com on 10 Aug 2012 at 2:28

GoogleCodeExporter commented 9 years ago 
Fine, thanks for heads up!

Original comment by dmitry....@gmail.com on 10 Aug 2012 at 2:44

GoogleCodeExporter commented 9 years ago 
P.S. found a bug in your latest patch (where 1 was still subtracted from 
tweensCount in the for loop), and just committed again with it fixed

Original comment by daniele....@gmail.com on 10 Aug 2012 at 2:57

GoogleCodeExporter commented 9 years ago 
Ouch, looks like I forgot to remove it, sorry, will be more accurate next time.

Original comment by dmitry....@gmail.com on 10 Aug 2012 at 2:59

GoogleCodeExporter commented 9 years ago 
Finished touching stuff

P.S. I don't want to harass you, so if you're not working on HOTween just 
ignore me - I'm a nerdily precise guy, so I'll go on reporting here when I 
start/stop changing stuff ;)

Original comment by daniele....@gmail.com on 11 Aug 2012 at 11:34

GoogleCodeExporter commented 9 years ago 
Haha, thanks for keeping me informed!

Original comment by dmitry....@gmail.com on 11 Aug 2012 at 11:37

GoogleCodeExporter commented 9 years ago 
BTW, what coding style do you prefer? I see both Allman and K&R brackets 
placing styles. So which is yours?)

Original comment by dmitry....@gmail.com on 11 Aug 2012 at 4:05

GoogleCodeExporter commented 9 years ago 
Anyway, catch an another patch.
I should mention there are not so much low-level optimizations could be done - 
many things already did with high performance target in mind what is great. I 
guess there are high-level optimizations are still possible but to make them I 
should know how the entire engine works and how it designed - I'm far from this 
ATM (

So this is mostly about "for" loops patch.
I'll investigate more in the "for" loops vs "for each" loops regarding to 
generic Lists though.
So far "for" loops were faster comparing to "for each" both on desktop and 
mobile, but I did tests with built-in arrays only.

Also I did few additional observations, please have a look at them:

Tweener:
The ForceSetSpeedBasedDuration() method has two loops inside.
I thought about merging them, but looks like they should be separated by 
design. So I'm not sure about this.

Sequence:
The Update method has two loops with different directions though. So I'm not 
sure if they could be merged too.

Different classes has repeating allocations that probably could be avoided 
using class variables:
PlugColor.DoUpdate - new Color
PlugRect.SetChangeVal - new Rect
PlugRect.SetIncremental - new Rect
PlugRect.DoUpdate - new Rect
PlugVector2.SetChangeVal - new Vector2
PlugVector2.DoUpdate - new Vector2
Same to the PlugVector3, PlugVector4
PlugQuaternion.DoUpdate - new Vector3
PlugVector3Path.DoUpdate - Vector3.up (new Vector3)

Also I did many changes in the OverwriteManager.AddTween method and since I 
can't check if it works I'd wish you to check my changes there.

ABSTweenComponent and some other classes has many "empty" properties like
public string id
{
    get
    {
        return _id;
    }
    set
    {
        _id = value;
    }
}

So them could be replaced by just a public class variables and improve 
performance a bit avoiding additional methods calls.

Same about read-only "empty" Properties like:
public bool destroyed
{
    get
    {
        return _destroyed;
    }
}
I guess they could be replaced by the public readonly vars for the same reason.

Please, let me know what do you think about all this.

Original comment by dmitry....@gmail.com on 11 Aug 2012 at 5:16

Attachments:

GoogleCodeExporter commented 9 years ago 
Great, thanks :) Gonna check everything tomorrow, but I'll have a busy weekend, 
so I might delay it until Monday, sorry.

About ForceSetSpeedBasedDuration(), I'm not sure, will have to check it.

About the Sequence Update instead, those loops do have to stay separate and 
"opposite", since that's the only way internal tweens are initialized correctly.

About the rest, I had mixed feelings about using "empty" properties, so I ended 
up making mixed logic. But you're right. I'll think about that too and 
eventually change them. Though I didn't get what you mean with readonly vars 
(if you're using readonly in C# terms). For example, "_destroyed" couldn't be 
made readonly, since it is changed after construction?

Original comment by daniele....@gmail.com on 11 Aug 2012 at 6:31

GoogleCodeExporter commented 9 years ago 
BTW, missed the previous comment. I definitely prefer Allman for classes and 
methods, KR for the rest. If you see Allman-ed ifs/fors/whiles it's because the 
other committer's Resharper reformatted the whole HOTween source code, and 
since then I'm trying to slowly bring it back to KR.

Sorry for the mess :P

Original comment by daniele....@gmail.com on 11 Aug 2012 at 6:39

GoogleCodeExporter commented 9 years ago 
Hey, it's OK about Monday, don't hurry with it!

About readonly - yeah, you're right, I've forgot those values are changed 
internally, so the only way to prevent them from external changes is get 
Property.

About coding style - it's good to know your preferences, will keep it in mind. 
And it looks not so messy now, all fine, it could be read very well.

Thanks for all your answers!

PS: Did you have any thoughts on the high-level critical parts (like updates 
and initializers) optimizations ATM?

Original comment by dmitry....@gmail.com on 11 Aug 2012 at 6:47

GoogleCodeExporter commented 9 years ago 
Hey Dmitriy (it's DmitrIY right? I see it's ends with "Y" here instead than 
"IY" as in Unity forums), sorry I'm being late, will check everything tomorrow. 
All hell broke lose yesterday and finally settled this afternoon, so I had no 
time to check the updates. I wanted to actually test if storing a list count is 
faster in C#, since adding a local variable can slow things down.

I have many thoughts about high-level updates (like using extensions and 
allowing lambdas instead than property names - as suggested by stfx ages ago - 
which would make everything much faster), but they all involve a complete 
rewrite of the code. So I don't think much could be done until V2.

Original comment by daniele....@gmail.com on 13 Aug 2012 at 7:38

GoogleCodeExporter commented 9 years ago 
Hey, Daniele!
You're totally right about Dmitriy, I didn't know how to write my name 
correctly when I registered my gmail account, haha, and now it's near to 
impossible to change it)

>>I wanted to actually test if storing a list count is faster in C#
Let me assure you it's faster. This is a tests results I got:

"Heavy" content (some math, vector instantiation and Math.round call) in the 
loop body:

Desktop:
length outside loop: 1575 ms
length inside loop:: 2125 ms

Mobile (iPad 1):
length outside loop: 69205 ms (oO)
length inside loop:: 71554 ms (oO)

"Light" loop body content (only one row of code):

Desktop:
length outside loop: 640 ms
length inside loop:: 1005 ms

Mobile (iPad 1):
length outside loop: 7444 ms
length inside loop:: 7932 ms

All iterating List<int> with 10 000 000 items ten times.
And local variables are stored on the stack in C# afaik so they aren't affect 
GC..

Also I just few minutes ago ended with testing for vs foreach loops on the 
List<> collections. I got this results:

Desktop:
for-loop: 599 ms
foreach-loop: 1577 ms

Mobile (iPad 1):
for-loop: 7972 ms
foreach-loop: 11151 ms

All iterating List<int> with 10 000 000 items ten times.
I had tested it both with pretty simple (in few lines) and more heavy code in 
the loop body. The tests results above is for the pretty simple content, for 
the more heavey content loop overhead becomes not so noticable, but for-loop 
sill spends less time. So I guess you could replace most of the for-each loops 
by the classic for-loops and probably get some little speed improvements. I'd 
like you to check it yourself before changing it though since my tests are 
based on the abstract data and code, but you could do all this tests right on 
the optimization target (HOTween sources) and see if it will be effective.

I'll keep testing further, since I still have many questions on performance of 
different things in C#.

Regarding to the high-levels optimizations - I see now, thanks for the info, 
BTW how far from the V2 are you? Do you have any planned time period for it 
already?

Original comment by dmitry....@gmail.com on 13 Aug 2012 at 8:24

GoogleCodeExporter commented 9 years ago 
>>Let me assure you it's faster. This is a tests results I got:
Of course, this is only about "to target", like
for (int i = 0; i < someList.Count; i++)

In case looping "from target", like
for (int i = someList.Count - 1; i >= 0; i--)
performance is just the same as using local variable for the Count since int i 
= * happens only once.

Original comment by dmitry....@gmail.com on 13 Aug 2012 at 9:47

GoogleCodeExporter commented 9 years ago 
Hey Dmitriy,

I just committed your patch. And wow, I realize I was quite dumb in not 
considering the storage of lengths as an outer var. I read a lot about 
performance optimizations (especially when working with AS), but somehow I 
never happened on that - though thinking about it, it's quite logical (unless 
the list/array contains just a couple elements). Now I understand why foreach 
loops (with simple operations) were faster than for on my test system :B

Thursday I'll start going through the remaining foreach loops and changing them 
using your system.

About high-level optimizations, V2 is very far, and I still have no planned 
period. My plan is to finish Journeyballs (the game I'm working on) first, and 
then move to V2. But I'm also taking any commissioned job (almost) that happens 
on me, so I really don't know when that will happen. Also, I actually started 
developing V2 a while ago, but then thought it would be better to have HOTween 
in the open for a year, before doing a completely new version. This way, any 
feedback and stuff I learned in that timespan could go in V2.

Original comment by daniele....@gmail.com on 14 Aug 2012 at 3:13

GoogleCodeExporter commented 9 years ago 
Hey again, Daniele!

Glad to hear you're committed a patch! Hope all will be fine after that =)
It's also good to hear you'll replace for-each to the for loops.

Regarding to the V2 - it's ok, V1 is pretty cool and better got most of it 
before going to the V2 with all experience gained on the V1!

Original comment by dmitry....@gmail.com on 14 Aug 2012 at 4:30

GoogleCodeExporter commented 9 years ago 
By the way, gave you commission privileges as I said. Forgot about that when I 
committed the patch :)

Original comment by daniele....@gmail.com on 14 Aug 2012 at 4:45

GoogleCodeExporter commented 9 years ago 
Great, thanks a lot!

Original comment by dmitry....@gmail.com on 14 Aug 2012 at 4:56

GoogleCodeExporter commented 9 years ago 
Whew I was incredibly late, but in the last version I finally removed all 
foreachs and replaced them with fors and a stored count as you showed me :)

Original comment by daniele....@gmail.com on 28 Aug 2012 at 7:48

GoogleCodeExporter commented 9 years ago 
Yay! That's cool, thanks! I hope it will gain additional speedup for the heavy 
HOTween usage cases!

Original comment by dmitry....@gmail.com on 28 Aug 2012 at 8:07