Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Famous Engine - MOST STABLE RELEASE HERE #504

Open
roblav96 opened this issue Nov 10, 2015 · 36 comments
Open

Famous Engine - MOST STABLE RELEASE HERE #504

roblav96 opened this issue Nov 10, 2015 · 36 comments

Comments

@roblav96
Copy link

THIS IS NOT OFFICIAL

Throughout the week I've been experimenting with different versions of Famous Engine to find one that's bug free. All versions I tested were consuming an absurd amount of CPU usage. Except one.

http://code.famo.us/famous/0.5/famous.min.js

That one there has been proven to be by far the most stable release I could find. I'm absolutely in love with this engine and I can't believe development has been put to a hult. For anyone having issues with the engine, just use this one. Thank you.

@CompSciFutures
Copy link

Hi Rob,

I think we are working on the same problem, this info is definitely useful.

Did you perchance make a test harness for experimenting with this that you
can share with me? I've been working on a performance problem that sounds
similar, might have a fix.

ap

On Tue, Nov 10, 2015 at 1:33 PM, Rob Laverty notifications@github.com
wrote:

THIS IS NOT OFFICIAL

Throughout the week I've been experimenting with different versions of
Famous Engine to find one that's bug free. All versions I tested were
consuming an absurd amount of CPU usage. Except one.
http://code.famo.us/famous/0.5/famous.min.js

That one there has been proven to be by far the most stable release I
could find. I'm absolutely in love with this engine and I can't believe
development has been put to a hult. For anyone having issues with the
engine, just use this one. Thank you.


Reply to this email directly or view it on GitHub
#504.

@roblav96
Copy link
Author

It all started out when I pushed my app to iOS development stage and noticed extremely high CPU idle usage in the xCode monitoring suite. I was using the most recent 0.7.1 release in my browserify build. I started searching for any official stable release and came across this one on the FAQ page.

Sure enough, after converting everything to a global build, everything worked amazingly!

@CompSciFutures
Copy link

Hi Rob,

Thanks for the clarification, my experiences are much the same. I'll take a
look at 0.5.

ap

On Mon, Nov 16, 2015 at 7:32 PM, Rob Laverty notifications@github.com
wrote:

It all started out when I pushed my app to iOS development stage and
noticed extremely high CPU idle usage in the xCode monitoring suite. I was
using the most recent 0.7.1 release in my browserify build. I started
searching for any official stable release and came across this one on the
FAQ page.

Sure enough, after converting everything to a global build, everything
worked amazingly!


Reply to this email directly or view it on GitHub
#504 (comment).

@roblav96
Copy link
Author

I have noticed that using the 0.5 version here in a browserify bundle still persists the problem. I would say just use the one I posted and have clarity knowing it will work. I've done a lot of testing with this and I am sure of the stability of this one.

@Arnleif
Copy link

Arnleif commented Jan 21, 2016

Hi.

I'm not sure if this is related to your slowdown, but it could be.

in core/Transform.js multiply (out, a, b) function (In Famous, latest version)

there are lines like this:
changed = changed ? changed : out[0] === res;

which is wrong as changed will be true if there were no changes.
out[0] !== res;
would be correct.

This bug leads to all nodes/transforms having their update and/or (?) transform calculation called every frame.

Looking at the same file in 0.5, that function does not exist. It seems to have been introduced in 0.6

A

@roblav96
Copy link
Author

How might one go about fixing this problem??? I would love to take a peek at it.

Are you saying all of the out[x] !== res?

@roblav96
Copy link
Author

I have noticed a slight boost. I'm gonna profile it and see.

@CompSciFutures
Copy link

Took a quick look and have observed this change reducing the size of the famous.core.FamousEngine._messages array by ~50%

@roblav96
Copy link
Author

Confirmed here as well!

@Arnleif
Copy link

Arnleif commented Jan 21, 2016

Yeah, all of those out[x] !== res in the multiply function.

But there are more problems as those compares are exact and since some positions sometimes are floaty, the compare still comes out as 'changed'.

In one of my cases the one of the out[x] is 1085 and res 1085.0000305175781
And everything should be standing still..

But there must be other things also spending the cpu as the 0.5.2 version uses 0-2ms on while the 0.7 version uses 20-30 ms with my fix..

@roblav96
Copy link
Author

How were you able to build 0.5.2 from npm? I get dependency errors as one of them were deleted from npm.

@speigg
Copy link

speigg commented Jan 21, 2016

Just curious: is anyone planning on actually forking famous & maintaining these fixes? I already know about https://github.com/infamous/famous-engine, but that fork is not being maintained.

@ManuelOverdijk
Copy link

@speigg have a look at https://github.com/dmvaldman/samsara

@arunabhdas
Copy link

@roblav96 Yes I agree that the 0.5.x release is the most stable version. It definitely make sense to continue this project and see it through till completion.

@speigg
Copy link

speigg commented Jan 21, 2016

@ManuelOverdijk Yes, I know about samsara. But that's based on famous 0.3 (pre-mixedmode), which was really entirely different (no webgl, no decoupling between the scene graph ui thread and the rendering pipeline, etc).

@roblav96
Copy link
Author

@ManuelOverdijk I have seen this work before. Very excited to see where this one goes! Seems like quite a lot of work has gone into it.

Anyone know how to build the 0.5.x via npm/browserify?

@Arnleif
Copy link

Arnleif commented Jan 22, 2016

npm run build-debug
npm run build-min

@roblav96
Copy link
Author

@Arnleif this will produce an error of a package removed from npm

@Arnleif
Copy link

Arnleif commented Jan 25, 2016

@roblav96 no errors here, only get an error if doing npm run build as that 'alias' does not point at build-min.

@roblav96
Copy link
Author

Hmm, weird :/

I've actually been switching my project to use Samsara.

https://github.com/dmvaldman/samsara

Works extremely well with Vue.js data binding and its very modular.

@Arnleif
Copy link

Arnleif commented Jan 25, 2016

Doing some changes to the 0.7.1 version, I've managed to get it down to using 0-2ms while the app is 'idle' on ipad2. Not sure if anyone is interested or where to post it. The fix is mostly about doing less lookups in transformsystem.update and sizesystem.update when nothing has changed.

@roblav96
Copy link
Author

yes id still be very interested in this!

@roblav96
Copy link
Author

just make a fork and post it as an issue in this repo like i did :D

@speigg
Copy link

speigg commented Jan 25, 2016

@Arnleif 👍

@CompSciFutures
Copy link

@Arnleif: nice work! Just post a diff/patch here if that's easier

@Arnleif
Copy link

Arnleif commented Jan 26, 2016

After some swearing.. (first time on github)
https://github.com/Arnleif/engine/commits/master

Note that the 'fix' has an assumption about some breakpoint and worldpos calculations. See the commit message.
I also found and fixed a bug when removing childs in Dispatch.prototype.dismount. Only every even (0,2,4,6..) child would get removed.

Note: I've noticed that there must be more bugs that lead to performance issue. Guess I have to dig for those also.

@roblav96
Copy link
Author

Looks great! I can already notice a great boost in performance. Thank you!

@CompSciFutures
Copy link

@Arnleif: legend. Will have a play on the weekend.

@dmvaldman
Copy link

Thanks for the shoutout to Samsara (https://github.com/dmvaldman/samsara) @ManuelOverdijk. It's true that Samsara began at Famous v0.3, but it has evolved quite a way since that. I hope some of you take the time to check it out. I'm here if you have any questions.

@roblav96
Copy link
Author

@dmvaldman I can vouch that he's been an amazing help with helping me get a grasp on the samsara techniques. Another big thing that grew my confidence in the project is this:

Formerly Chief Architect @befamous, PhD in math

@dmvaldman
Copy link

💪

@CompSciFutures
Copy link

@Arnleif: I have been playing around with integrating your performance fixes. A couple things broke some of my existing famous code, will need to spend some more time on it next weekend to go through each of your changes.

This branch has your code merged in with a few other fixes:
https://github.com/aprender/ReFamous/tree/Arnleif-performance-fixes

Question - which profiler did you use? I noticed you left some calls to it in your code (stats.profileStart() / stats.profileEnd()).

ap

@Arnleif
Copy link

Arnleif commented Feb 2, 2016

The profiling code I use is a mix between threejs stats.min.js (fps/memory view) and some timing code I added to it.

https://dl.dropboxusercontent.com/u/474653/fpscounter.js

And then you init and do something like this in your update loop. The fpscounter is very simple and only show each frames times directly. No smoothing, no peaks. On ipad (2), the timing is only down to milliseconds so it gets harder profiling smaller portions of code.

Init()
{
    stats = new FpsCounter();

    stats.domElement.style.position = 'absolute';

    stats.domElement.style.top = '440px';
    container.appendChild(stats.domElement);

}


Update() 
{
    if(stats) stats.ProfileStart("All")

    if(stats) stats.ProfileStart("Scene");
    scene.update();
    if(stats) stats.ProfileEnd("Scene");

    something.more();


    if(stats) stats.ProfileEnd("All")

    if(stats) stats.ProfileEnd("Frame")
    if(stats) stats.update();
    if(stats) stats.ProfileStart("Frame")
}

@Arnleif
Copy link

Arnleif commented Feb 4, 2016

@Aprender I found an issue with my transform optimizing. Commited a fix for it.
https://github.com/Arnleif/engine/commits/master

@CompSciFutures
Copy link

@Arnleif I have taken up most of your changes in https://github.com/aprender/ReFamous/tree/develop

The Size.js & SizeSystem.js changes have the same bug that you fixed in Opacity & TransformSystem - can you please post a fix?

You have a few unrelated changes included in the same commit, while other changes are spread across multiple commits. I have reorganized your work to allow us to mute any change by reverting any commit in any order, I'm hoping I got it right.

The improvement is marginal right now (~25%), but I imagine that is because the SizeSystem is still causing lots of updates. Lets see what happens once your SizeSystem fix goes in and a couple others that I have been working on are all in the same code base.

ap

@Arnleif
Copy link

Arnleif commented Feb 15, 2016

Hi. I think the fix you want has already been commited:
Arnleif@e6131c1

Maybe you missed those two files somehow? Size.js and SizeSystem.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants
@arunabhdas @speigg @dmvaldman @roblav96 @ManuelOverdijk @CompSciFutures @Arnleif and others