-
Notifications
You must be signed in to change notification settings - Fork 131
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
getProps + Context performance #346
Comments
I really like that approach. That was the hard part about optimizing the renderer, context was a free-for-all, so knowing when to re-render was pretty tricky. I'm ok with implementing this API in 2.0. We're not doing any The next piece of optimization I'm thinking about is with local state, and it ties in with this. If a component deep in the tree stores some local state on the context, which then triggers a re-render, we can't skip whole sub-trees. The parent might not be "dirty" but the child is because it's relying on something from the context. I'm not sure about the best way to do this. Elm gets around this by not having "context" as something special, you just pass it down like any other param. It also doesn't optimize the rendering as much. I think the first approach will be to use the logic you mentioned, and then traverse down the tree to any leaf component nodes, then check them too. Also, you could also have an API like this now: const getProps = (attrs, children, path, context) => {
return { ...attrs, user: context.users[props.userId] };
};
const render = ({ user }, dispatch) => {
return <li>{ user.firstName } { user.lastName }</li>
};
export default { getProps, render } It seems pretty verbose, but you get a really clear idea of what your |
the bonus in this, as we discussed in a different thread, is that |
Great!
Exactly. One other good thing with this approach is the freedom it will give to users with using context: they will be able to place the cursor between a minimalist context with lots of derived data and a very explicit context, without having to worry much about performance impact. |
Sounds perfect to me! |
And it probably won't be that hard to implement either. Which is an extra bonus. It's niceties like this that make it much more beneficial to use this over raw virtual-dom. |
Is this going to be implemented? If so, I'd like to prototype it over at decca. :-) |
Also, I think it'd be nice if getProps({ attrs, path, context, dispatch })
onCreate({ attrs, path, context, dispatch })
onUpdate({ attrs, path, context, dispatch }) /* 'attrs' is what previously was 'props' */
render(props, dispatch) (sorry, can't remember the exact stuff off the top of my head) |
Render needs children, don't know if lifecycle methods would benefit more from attrs or props (maybe both)? |
+1 from me on EDIT: On second thought, this doesn't seem like it'd actually be all that performant. You are still doing an O(n) operation where n is the total number of components rendered, as far as I can tell - you aren't able to actually ignore any sections of the tree completely, you just exclude the |
It will be performant if There is no scalability issue. If you use |
@troch Ya, but right now all components have access to context. Would you not pass context to all components? You maybe could say that if a component doesn't export a |
If a component needs context, it exports a |
@troch Maybe I am missing something...Let me give some examples. If I have:
Are those two examples correct? If so, contrast that with immutable re-rendering, where at each level of the tree, if the props haven't changed as determined by a shallow equality check, you simply stop descent and only descend into subtrees where there is a new value. That rendering style scales fine to a million components on a page, or a billion - all that matters is how many things change at a time. Whereas the performance profile of |
So for your two examples:
Immutability and shallow equality check is not enough for knowing if you can bail out an entire part of your tree, because of the use of context by components:
From this two simple observations, the problem is to be able to account for context use. Example Let's consider we have a list of items to display, and each item can be selected. In context, we will have the list of items + the list of selected IDs: const context = {
items: [
{
id: 'A',
description: 'A beautiful renaissance painting from an unknown italian painter',
imageUrl: '//mycdn.com/painting.png' ,
},
{
id: 'B',
description: 'The complete discography of Bruce Springsteen, including some never released tracks',
imageUrl: '//mycdn.com/discography.png'
},
{
id: 'C',
description: 'A pair of muddy welly boots',
imageUrl: '//mycdn.com/wellies.png'
},
{
id: 'D',
description: 'A black leather sofa with comfortable armrests',
imageUrl: '//mycdn.com/sofa.png'
}
],
selectedItemIds: ['A', 'C']
} Tree of components We have two connected components (with
Your tree looks like this: const visibleItemIds = ['A', 'B', 'C'];
<ItemList visibleItemIds={visibleItemIds} />
const getProps = (context, attrs) =>
({ ...attrs, items: context.items });
const render = ({ props }, dispatch) => {
const { items, visibleItemIds } = props;
const visibleItems = items.filter(item => visibleItemIds.includes(item.id));
return ul({}, visibleItems.map(item => li(Item, { item }));
}
<Selected id={ item.id } /> const getProps = (context, attrs) =>
({ ...attrs, selectedItemIds: context.selectedItemIds });
const render = ({ props }, dispatch) => {
const { id, selectedItemIds } = props;
const isSelected = selectedItemIds.includes(id);
return div({}, isSelected ? 'Selected' : '');
} Updates If the list of selected items changes:
If the list of visible items changes ('D' becomes visible)
|
Ya, I understand that context makes the immutable approach impossible. I guess I should have been more clear: I was suggesting that maybe the fact that context makes the immutable subtree skipping optimization impossible means that context is too powerful an abstraction, and that a more limited approach might make sense. virtex-local for instance, solves the local state problem using a slightly less powerful abstraction than a general context, and stores the state as a tree, rather than a flat map. Given those two things, you can directly memoize a component by its props/state. Maybe in practice few enough components will use side-loaded data / local state, or maybe |
I think we are talking about roughly the same thing. |
I think I misspoke slightly. You can memoize not just the component by its props/state, but its entire subtree if you don't have a general context. |
I have played with a Not sure the log below is understandable, I find it very difficult to name things around trees, nodes, elements, etc... But first build 4-5ms, change of props from the top 1ms, change of context affecting the top 1ms. I need to add change of context affecting leaves. |
@troch What does the DOM tree look like in that test? Render times don't make much sense absent that context. |
It seems like a lot of the complexity goes away if we don't have context. It's mostly just for convenience and potentially breaks the purity. Do we think the convenience is worth the potential performance hit? At the very least we'd need to shallow check each subtree for changes rather than completely bailing. I'm starting to lean towards not having context and just keeping it simple. |
@anthonyshort I think i'm in favor of that as well. The main downside then is that local state becomes substantially more explicit, although that may be ok. I think it makes sense to think through what it would look like. Without any sort of context at all, you'd need to use something like baobab, or perhaps some lighter weight state tree. What i'd really like is a state lens / tree thing that works like this: function render ({state}) {
return <Dropdown state={state.get('dropdown')} />
}
function Dropdown ({state}) {
return <div onClick={() => state.set('open', true)} />
} Where If you add a EDIT: The trickiest part is going to be deciding when to destroy the local state data. Obviously you'd want to do it |
As long as it is can scale and works with non data-based changes (animations or intervals), I'm fine with it. |
I made a quick demo of the idea above: state-lens. It's probably not the optimal way of implementing it, but it works to play around with. |
I strongly believe that we should have no context. What’s wrong with having an abstraction to pass down context as props? |
Wood, here we go: https://github.com/rstacruz/deku-stateful Simple higher-order component that should work almost anywhere. |
Hmm...in thinking more about this, what if rather than a function observe ({path}) {
return 'states[' + path + ']'
}
export {
observe,
render
} Deku would then internally do something like this: const paths = component.observe(model)
paths.forEach(path => listen(path, rerenderSubtree(model.path)) Where listen works something like this: const listeners = {}
let prevState
store.subscribe(() => {
const nextState = store.getState()
traverse(nextState, prevState, '')
prevState = nextState
})
function traverse (nextState, prevState, path) {
for (let key in nextState) {
const subpath = path + '.' + key
const nextVal = nextState[key]
const prevVal = prevState && prevState[key]
if (prevVal !== nextVal) {
if (listeners[subpath]) listeners[subpath].forEach(fn => fn(nextVal, prevVal, key))
if (isObject(nextVal)) traverse(nextVal, prevVal, subpath)
}
}
}
function listen (path, cb) {
listeners[path] = listeners[path] || []
listeners[path].push(cb)
} Since the state tree is immutable, this would be relatively performant, since it would only explore the subtrees of state that changed. Since the components are not able to do arbitrary transformations of the data, you only have to traverse the state tree to see what has changed, so the number of operations you have to perform is not proportional to the number of components rendered. This would get you essentially the same performance as observable-based systems while maintaining the ergonomics and appearance of top-down component rendering. It would also allow |
Hi,
So the new version of deku coincides with me writing a blog article about property injection in React (still a draft), and I gave recently a lot of thoughts to UI trees of components.
I really like the new deku, I think you are spot on in the paradigm of functional UIs (not talking about functional and reactive ones): passing a context, a dispatch function, etc...
Where I see room for improvement is in rendering optimizations. I have tried several ways with the current API but they were not a good idea or not universal friendly (mostly both). I think any optimization should be done by the renderer, and not be component-land as components should remain as much as possible agnostic of the environment they are rendered in. The other issue preventing optimizations is the fact components can tap freely into the context without the renderer being aware of it.
I see a tree as a set of nested subtrees. A subtree:
The first key thing to do is to prevent free use of context, by removing it from the model, and explode a render function into two parts:
By default,
getProps
is({ props }, context) => props
. It is easy for the renderer to check if anything has changed, it is also a great place for optimizing computation of derived data (by using memoization and something like reselect).This is very efficient if immutability of props is enforced. The way to re-render a set of subtrees is to identify subtree top nodes, and hop from subtree to subtree (checking for change). Marshalling the component nodes:
model.props !== getProps(model, context)
)The text was updated successfully, but these errors were encountered: