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

IncrementalPreview #4953

Closed
wants to merge 1 commit into from
Closed

IncrementalPreview #4953

wants to merge 1 commit into from

Conversation

sahrens
Copy link
Contributor

@sahrens sahrens commented Dec 23, 2015

Preview of new experimental Incremental concept for feedback. I have D2506522 internally that is the source of truth and necessary InteractionManager changes that are in the process of landing.

The problem addressed here is expensive components that can tie up the JS thread during render, making the app unresponsive to taps, the canonical example being scroll loading of expensive rows. The approach is to slice up the rendering into separate chunks of work separated by setTimeout calls.

Everything wrapped in <Incremental> is rendered sequentially via InteractionManager.
The onDone callback is called when all descendent incremental components have
finished rendering, used by <IncrementalPresenter> to make the story visible all at once
instead of the parts popping in randomly.

This includes an example that demonstrates streaming rendering and the use of
<IncrementalPresenter>. Pressing down pauses rendering and you can see the
TouchableOpacity animation runs smoothly. Video:

https://youtu.be/4UNf4-8orQ4

Plan is to use this in WindowedListView and get that out as an Experimental component in open source soon as well.

Note that the new setDeadline method must be called on InteractionManager, otherwise everything will just get batches up in setImmediate. We should consider changing that default.

Ideally this will be baked into React Core at some point, but according to @jwalke that's
going to require a major refactoring and take a long time, so going with this for now.

Preview of new experimental Incremental concept for feedback.  I have D2506522 internally that is the source of truth.

Video:
https://youtu.be/4UNf4-8orQ4
@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Dec 23, 2015
@brentvatne
Copy link
Collaborator

🚀 🎸

@dsibiski
Copy link
Contributor

Oooooh! 😍

@brentvatne
Copy link
Collaborator

My thoughts (as @sahrens and I discussed elsewhere) -- it would be neat to find a way to integrate requestIdleCallback (perhaps optionally) in order to further de-prioritize incremental rendering, so incremental chunks only start when there is some 'idle' time. This could be optional.

@chandu0101
Copy link
Contributor

does this solve js navigator transitions issue ..?

@brentvatne
Copy link
Collaborator

@chandu0101 - it could help if your views in the new scene are all inside of incremental presenters. there is other work being done to specifically address that right now though -- offloading animations to main thread.

@chandu0101
Copy link
Contributor

it could help if your views in the new scene are all inside of incremental presenters. there is other work being done to specifically address that right now though -- offloading animations to main thread.

cool ok , thanks for info :)

@ssssssssssss
Copy link

🚀 🎸 Does the experimental WindowedListView solves the memory hungry issue for a long list?

@sahrens
Copy link
Contributor Author

sahrens commented Dec 25, 2015

Yes, WindowedListView is focused on the memory usage of infinite lists, but there are tradeoffs, especially because it is designed for variable height rows.

On Dec 24, 2015, at 5:25 PM, fxwan notifications@github.com wrote:

Does the experimental WindowedListView solves the memory hungry issue for a long list?


Reply to this email directly or view it on GitHub.

@Kureev
Copy link
Contributor

Kureev commented Dec 25, 2015

Oh, sounds amazing! 😻

@facebook-github-bot
Copy link
Contributor

@sahrens updated the pull request.

@deanmcpherson
Copy link
Contributor

Hey @sahrens, I'm working on updating an internal tab bar component, which I'm have transition speed issues with complex views on iPad (in particular unmount component speed of views containing multiple list views). Is Incremental a good fit for improving this kind of perf? WindowedListView sounds like it may help with multiple listviews as well.

@sahrens
Copy link
Contributor Author

sahrens commented Jan 11, 2016

Doubt it would help with unmount - any idea why that's slow?

This might help with transition smoothness, but doesn't help with total speed. It's goal is to improve responsiveness, so if you touch the screen during a big render, the app can respond immediately rather than waiting for that render to complete - would that help what you're seeing?

On Jan 10, 2016, at 9:09 PM, Dean McPherson notifications@github.com wrote:

Hey @sahrens, I'm working on updating an internal tab bar component, which I'm have transition speed issues with complex views on iPad (in particular unmount component speed of views containing multiple list views). Is Incremental a good fit for improving this kind of perf? WindowedListView sounds like it may help with multiple listviews as well.


Reply to this email directly or view it on GitHub.

@facebook-github-bot
Copy link
Contributor

@sahrens updated the pull request.

1 similar comment
@facebook-github-bot
Copy link
Contributor

@sahrens updated the pull request.

@bestander bestander closed this Mar 1, 2016
@bestander bestander deleted the sahrens-incremental-preview branch March 1, 2016 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants