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

[Not for commit][Fiber] Sierpinski Triangle Demo #7180

Closed
wants to merge 3 commits into from

Conversation

sebmarkbage
Copy link
Collaborator

Just a dump of a demo experiment if someone wants to follow along.

Found some minor bugs that needs proper fixes.

Renders fractal triangles with numbers updating each second in each terminal. The whole thing is also scaled through an rAF animation.

Useful demonstrating various starvation issues and tradeoffs. (Initial render is starved so you need to switch tabs once before the demo properly starts. :) )

return;
}
if (typeof newProps.style === 'object') {
Object.assign(domElement.style, newProps.style);
Copy link
Collaborator Author

@sebmarkbage sebmarkbage Sep 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the demo I intentionally avoid this Object.assign. This operation is slow enough that you'll drop frames. 731 updates x 0.02ms for this operation is enough. That's on a powerful MacBook desktop. Let alone mobile.

It is absolutely critical that the commit path is super optimized and any slow work have to be done in the prepare step.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.6%) to 86.667% when pulling 17c328f on sebmarkbage:fiberexample into 839697f on facebook:master.

@sebmarkbage sebmarkbage force-pushed the fiberexample branch 2 times, most recently from 42fd085 to 74e4141 Compare September 8, 2016 00:47
}
leave() {
this.setState({
hover: false
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@acdlite This example doesn't actually work properly right now with the rebased setState. Something doesn't update properly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It causes the props to revert to their initial value when they were first rendered. Something must be screwing up the reuse since the original props shouldn't be around for that long.

We don't want to use the "current" props/state because if we have
started work, then pause and continue then we'll have the newer
props/state on it already. If it is not a resume, it should be the
same as current.
Currently we flag the path to a setState as a higher priority
than "offscreen". When we reconcile down this tree we bail out
if it is a hidden node. However, in the case that node is already
completed we don't hit that bail out path. We end up doing the
work immediately which ends up committing that part of the tree
at a higher priority.

This ensures that we don't let a deprioritized subtree get
reconciled at a higher priority.
Adds some debug information to ReactDOMFiber
@acdlite
Copy link
Collaborator

acdlite commented Dec 13, 2016

Here's a video of the example in action: https://www.facebook.com/acdlite/videos/10205857503795058/

@quantizor
Copy link
Contributor

@sebmarkbage @acdlite it'd be cool if there was a way to build this automatically and post it somewhere so people can follow along and see the code in action

@sebmarkbage
Copy link
Collaborator Author

Already landed in another PR.

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

Successfully merging this pull request may close these issues.

4 participants