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

getProps + Context performance #346

Open
troch opened this issue Jan 3, 2016 · 26 comments
Open

getProps + Context performance #346

troch opened this issue Jan 3, 2016 · 26 comments

Comments

@troch
Copy link

troch commented Jan 3, 2016

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:

  • receives properties from its owner (except if it is the root node), and gets other properties injected (window, state, etc...)
  • share the same data dependency and is pure (passing props down from its top node).

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:

  • A computing props part, where context can be sliced, emulated local state injected and derived data computed
  • A rendering part, transforming props to virtual elements
const getProps = (model, context) => {
    const { props } = model;
    return { ...props, user: context.users[props.userId] };
};

const render = ({ props }) => {
    const { user } = props;
    return <li>{ user.firstName } { user.lastName }</li>
};

export default { getProps, render }

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:

  • is this component modifying props ( model.props !== getProps(model, context) )
    • Yes -> Shallow compare props, are they different?
      • Yes -> New subtree, re-render component and go to next component node
      • No -> Skip to next component modifying props
    • No -> Skip to next component modifying props
@anthonyshort
Copy link
Owner

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. getProps would work similar to the connect function in Redux. It felt a little dirty having that in the render function.

I'm ok with implementing this API in 2.0. We're not doing any shouldUpdate or checks on components at the moment to improve rendering. This seems like the way to do it.

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 props will look like with no magic.

@rstacruz
Copy link
Collaborator

rstacruz commented Jan 4, 2016

the bonus in this, as we discussed in a different thread, is that render can be more straight-forward-ly memoized. neat.

@troch
Copy link
Author

troch commented Jan 4, 2016

Great!

getProps would work similar to the connect function in Redux

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.

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.

@anthonyshort
Copy link
Owner

Sounds perfect to me!

@anthonyshort
Copy link
Owner

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.

@rstacruz
Copy link
Collaborator

rstacruz commented Jan 4, 2016

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 }

Is this going to be implemented? If so, I'd like to prototype it over at decca. :-)

@rstacruz
Copy link
Collaborator

rstacruz commented Jan 4, 2016

Also, I think it'd be nice if getProps received the same model as the lifecycle hooks:

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)

@troch
Copy link
Author

troch commented Jan 4, 2016

Render needs children, don't know if lifecycle methods would benefit more from attrs or props (maybe both)?

@ashaffer
Copy link

ashaffer commented Jan 6, 2016

+1 from me on getProps. You could also use it for validation or something if you wanted.

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 render and the diff. This would scale significantly worse than a React-style shouldUpdate.

@troch
Copy link
Author

troch commented Jan 6, 2016

It will be performant if getProps just selects what it needs from context, you can leave expensive stuff to the render function. Once your props are selected out of attributes and context, you can do an implicit shouldUpate by shallow comparing props with nextProps.

There is no scalability issue. If you use shouldUpdate to bail an entire parts of your tree, you don't have an O(n), but you also need to have components being accountable for their children data dependencies, meaning a component can be the only source of data for its children. So you don't pass context around (or you pass it as a prop).

@ashaffer
Copy link

ashaffer commented Jan 6, 2016

@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 getProps then it doesn't receive context - but if you did that, then only subtrees with no local state would be able to skip re-renders, and in most applications there is a lot of little transient state at the leaves, so that seems unlikely to be of much benefit. Or is there some aspect to this approach that i'm not understanding?

@troch
Copy link
Author

troch commented Jan 6, 2016

If a component needs context, it exports a getProps. A leaf is part of a subtree, or a subtree itself if it has its own getProps. When updating a leaf you'll need to traverse all the tree, getProps are some sort of compulsory checkpoints from root to leaves, allowing you to reach them quicker (if they still exist). It is a dirty checking process, as opposed to marking nodes dirty with setState. To me it sounds like a very reasonable performance optimization, especially for what you call little transient states at the leaves.

@ashaffer
Copy link

ashaffer commented Jan 6, 2016

@troch Maybe I am missing something...Let me give some examples. If I have:

  • N components and each of them exports a getProps. Then on every re-render, getProps needs to be called N times.
  • Several subtrees, all of which have a getProps exporting component at their bottom-most leaf, then every component (with the possible exception of that leaf component if nothing has changed) must have its render re-evaluated each time.

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 getProps seems to be proportional to how many components contain a getProps in their subtree (which, I would think, would end up being most components - making re-renders ~O(n))

@troch
Copy link
Author

troch commented Jan 6, 2016

So for your two examples:

  • That would be very naive, and not real-world like
  • That is more like a possible application, except not all nodes have to be re-rendered in order to reach a leaf

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:

  • For a component, props can be the same, but something in context might have changed and you would prevent that component to re-render despite the context change
  • Same thing for components down stream: they can freely use context, and as a component upstream I am not able to understand their data dependencies, because they don't only depend on what I will pass down to them.

From this two simple observations, the problem is to be able to account for context use. getProps is a way to control the use of context and for components to declare their dependencies. Then you can shallow compare the resulting props (after injecting some slices of context into your props: attrs + context -> props).

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 getProps):

  • ItemList: receives a list of visible item IDs as props, and need to get the list of items from context
  • Selected: receives an item ID, get the list of selected item IDs from context

image

Your tree looks like this:

const visibleItemIds = ['A', 'B', 'C'];
<ItemList visibleItemIds={visibleItemIds} />

ItemList would look like that:

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 would look like this:

<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:

  • ItemList.getProps evaluates, resulting props are shallow compared with previous ones. No changes, so all nodes below them are bailed out, except the ones which are not 'pure' (loading data sideways).
  • Each Selected.getProps is evaluated and they re-render. Note: here we could move the computation of isSelected is getProps in order to shallow compare { isSelected }.

If the list of visible items changes ('D' becomes visible)

  • ItemList.getProps evaluates, resulting props are shallow compared and different. Nodes below (Item) have their props / nextProps shallow compared --> the 3 existing ones have not changed so we can bail out all nodes below them except connected ones.
  • The new Item node is rendered
  • The 3 existing Selected.getProps evaluate, resulting in no call of render.

@ashaffer
Copy link

ashaffer commented Jan 6, 2016

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 getProps is fast enough that it won't be an issue, but it seems much less obvious that it will be performant for large scale apps.

@troch
Copy link
Author

troch commented Jan 6, 2016

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

I think we are talking about roughly the same thing.

@ashaffer
Copy link

ashaffer commented Jan 6, 2016

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.

@troch
Copy link
Author

troch commented Jan 6, 2016

I have played with a getProps / context slicer approach (just building and rebuilding a tree, not diffing two trees together for patching and not using deku).

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.

image

@anthonyshort anthonyshort changed the title API Proposal getProps + Context performance Jan 7, 2016
@ashaffer
Copy link

ashaffer commented Jan 8, 2016

@troch What does the DOM tree look like in that test? Render times don't make much sense absent that context.

@anthonyshort
Copy link
Owner

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.

@ashaffer
Copy link

ashaffer commented Jan 9, 2016

@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 state.get progressively lenses deeper into the tree and state.set returns an action that will be handled by something like redux-ephemeral. I considered doing something like that for virtex, but ultimately decided it would be annoying to have to explicitly pass state, but maybe it's not really so bad.

If you add a transformProps hook still too, you can recreate the elm architecture a bit by specifying a reducer to couple to the state cursor.

EDIT: The trickiest part is going to be deciding when to destroy the local state data. Obviously you'd want to do it onRemove, but having to manually add that everywhere seems kinda terrible.

@troch
Copy link
Author

troch commented Jan 9, 2016

As long as it is can scale and works with non data-based changes (animations or intervals), I'm fine with it.

@ashaffer
Copy link

ashaffer commented Jan 9, 2016

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.

@iclanzan
Copy link

I strongly believe that we should have no context. What’s wrong with having an abstraction to pass down context as props?

@rstacruz
Copy link
Collaborator

Wood, here we go:

https://github.com/rstacruz/deku-stateful

Simple higher-order component that should work almost anywhere.

@ashaffer
Copy link

Hmm...in thinking more about this, what if rather than a getProps which does an arbitrary transformation of state, we did more like an observe, which returned the paths that this particular component wanted to listen to. Observe would be called only once, on initialization.

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 rerenderSubtree to either rerender the entire subtree, or just the partial subtree - an optimization which could ideally be done transparently to components.

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

5 participants