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

Poor performance with multiple components which refresh on mount #155

Closed
brainkim opened this issue Sep 11, 2020 · 2 comments · Fixed by #200
Closed

Poor performance with multiple components which refresh on mount #155

brainkim opened this issue Sep 11, 2020 · 2 comments · Fixed by #200
Labels
performance Suboptimal speed or resource usage

Comments

@brainkim
Copy link
Member

brainkim commented Sep 11, 2020

One Crank idiom which I really like is components which refresh on mount:

function *Component() {
  this.schedule(() => this.refresh());
  const ref = yield <Child />;
  for ({} of this) {
    // do dirty imperative things
    yield <Copy />
  }
}

Unfortunately, if you have many child components which do this, each will call arrange with the parent host element, and complete on the renderer, which can degrade performance. I’m not sure if this means we should batch refreshes for the parent, but maybe that’s the solution. The bad thing about this is that it might mean the refresh of sync components has to be deferred, whereas I really would like to adhere to the pattern where sync components render synchronously. Something to think about.

@brainkim brainkim added the performance Suboptimal speed or resource usage label Sep 11, 2020
@brainkim
Copy link
Member Author

brainkim commented Jan 2, 2021

Making the “rearranging” step of refresh() calls happen asynchronously is tempting, but I think one possible solution is to detect synchronous refresh() calls which happen while a component is being updated and avoid running the rearranging logic when this is detected. I’m not sure if this would work, but it would be nice because it preserves the synchronous nature of refresh calls. Probably some kind of advanced logic with the IsUpdating flag. We need to make sure that async trees continue to trigger a rearrange, though then again, we would have the same problem when multiple async children components refresh and fulfill in the same microtask.

@brainkim
Copy link
Member Author

Fixed in 0.3.11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Suboptimal speed or resource usage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant