Quadratic slowdown with MobX on simple list example

[hyperknot]

I'm loading 1000 items from an API by pagination, and I would like to display them as they load.

My problem is that MobX is stalling my browser, even after a few hundred items.

I suspect the problem is that ever single list.push() triggers a full re-render. I don't know if it's a bug or it's supposed to work like this, but it results in quadratic times.

500 items => 2.5 seconds
1000 items => 10 seconds
2000 items => 40 seconds

(all this with 100% CPU usage).

Are there any ways to only trigger a re-render for every 50 or 100 items? I've even looked at the Debounced computed but it made no difference.

I believe I've followed the React Optimizations docs, but it's still slow. In my original code I'm using actions but it made no difference here.

From the docs it seems to me that MobX should be intelligent and only re-render the part of the DOM which requires it, not the full element on every list addition. Is this a bug then?

I've created a minimal reproducible example:
https://codesandbox.io/s/slow-mobx-react-qj1w9

I've also tried MobX dev tools but it didn't display anything to me.

import { configure, makeObservable, observable } from 'mobx'

import { observer } from 'mobx-react'
import React from 'react'
import ReactDOM from 'react-dom'

configure({
  enforceActions: 'never',
})

export class TodoList {
  @observable items = []

  constructor() {
    makeObservable(this)
  }
}

export class TodoItem {
  @observable id

  constructor() {
    makeObservable(this)
  }
}

@observer
class TodoListEl extends React.Component {
  render() {
    return (
      <div style={{ wordWrap: 'break-word' }}>
        {list.items.map((i) => (
          <TodoItemEl key={i.id} item={i} />
        ))}
      </div>
    )
  }
}

function TodoItemEl(props) {
  return props.item.id + ' '
}

const list = new TodoList()

ReactDOM.render(
  <React.StrictMode>
    <TodoListEl />
  </React.StrictMode>,
  document.getElementById('root')
)

for (let i = 0; i < 1000; i++) {
  const item = new TodoItem()
  item.id = i
  list.items.push(item)
}

[mweststrate]

please use actions instead of disabling them :)

…

On Mon, Jan 3, 2022 at 10:21 PM Zsolt Ero ***@***.***> wrote: I'm loading 1000 items from an API by pagination, and I would like to display them as they load. My problem is that MobX is stalling my browser, even after a few hundred items. I suspect the problem is that ever single list.push() triggers a full re-render. I don't know if it's a bug or it's supposed to work like this, but it results in quadratic times. 500 items => 2.5 seconds 1000 items => 10 seconds 2000 items => 40 seconds (all this with 100% CPU usage). Are there any ways to only trigger a re-render for every 50 or 100 items? I've even looked at the Debounced computed <#3013> but it made no difference. I believe I've followed the React Optimizations <https://mobx.js.org/react-optimizations.html> docs, but it's still slow. From the docs it seems to me that MobX should be intelligent and only re-render the part of the DOM which requires it, not the full element on every list addition. Is this a bug then? I've created a minimal reproducible example: https://codesandbox.io/s/slow-mobx-react-qj1w9 import { configure, makeObservable, observable } from 'mobx' import { observer } from 'mobx-react'import React from 'react'import ReactDOM from 'react-dom' configure({ enforceActions: 'never',}) export class TodoList { @observable items = [] constructor() { makeObservable(this) }} export class TodoItem { @observable id constructor() { makeObservable(this) }} @observerclass TodoListEl extends React.Component { render() { return ( <div style={{ wordWrap: 'break-word' }}> {list.items.map((i) => ( <TodoItemEl key={i.id} item={i} /> ))} </div> ) }} function TodoItemEl(props) { return props.item.id + ' '} const list = new TodoList() ReactDOM.render( <React.StrictMode> <TodoListEl /> </React.StrictMode>, document.getElementById('root')) for (let i = 0; i < 2000; i++) { const item = new TodoItem() item.id = i list.items.push(item)} — Reply to this email directly, view it on GitHub <#3247>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN4NBCHIINLMVTR3AGOZGTUUIOP5ANCNFSM5LGDOTZQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[hyperknot]

In the original app I had everything in an action, but I see what you mean, that I can wrap an action outside these smaller actions and then they'll be batched, is that correct?

So basically for a paginated page load, it'd look something like this?

  async loadPages() {
    for (let offset = 50; offset <= total; offset += 50) {
      const res = await get(...)
      await this.loadPage(res)
    }
  }

  @action
  async loadPage(data) {
    for (const row of data) {
      const item = new TodoItem()
      item.id = row.i
      this.addItem(item)  // <- also an action
    }
  }

So MobX only cares about the outermost action and does batching based on that?

[urugator]

Mobx only batches synchronous updates, you need a buffer for these 50 items, once it's full move the items from buffer to the actual observable array in a single synchronous operation.

[urugator]

From the docs it seems to me that MobX should be intelligent and only re-render the part of the DOM which requires it,

Mobx is intelligent about notifying react components, eg if array changes it notifies TodoListEl if only individual item changes, it notifies individual TodoItemEl, but we can't eg append element directly to DOM (or virtual DOM) if you add new item to array.

[hyperknot]

Thanks for the explanation.

So about the DOM, it'll always be a full re-render of the list, the right way with MobX then is to carefully control when are re-renders triggered right?

Mobx only batches synchronous updates
Can you explain this more? Is the problem that I'm in a JS async function? So the solution is to extract the sync part out of the async function into an action?

[urugator]

So about the DOM, it'll always be a full re-render of the list

If you add/remove items to array, yes. It's a limitation of react's DOM diffing approach. You need a virtualized list eg: https://github.com/bvaughn/react-virtualized

carefully control when are re-renders triggered

Usually you don't have to be very careful about that. Sync mutations are batched by action and async operations are slow (compared to JS), so usually you DO want to update UI eg in between network requests.

Can you explain this more?

#3171 (reply in thread)

[urugator]

Actually @mweststrate is working on something that could eventually help :)
https://portal.gitnation.org/contents/beyond-virtual-lists-how-to-render-100k-items-with-100s-of-updatessec-in-react

[hyperknot]

I rewrote the small sample using a real world JSON API pagination. It works with 1 re-render per API fetch. Is this good code-wise, or you'd recommend me to do something different? How would you use flow here? I'm still reading into flow.

https://codesandbox.io/s/mobx-api-async-3mcuv

import { action, makeObservable, observable } from 'mobx'
import { observer } from 'mobx-react'
import React from 'react'
import ReactDOM from 'react-dom'

function sleep(sec) {
  return new Promise((resolve) => setTimeout(resolve, sec * 1000))
}

class TodoList {
  @observable items = []

  constructor() {
    makeObservable(this)
  }

  async fetchPages() {
    for (let i = 1; i <= 40; i++) {
      const url = `https://jsonplaceholder.typicode.com/photos?_page=${i}&_limit=50`
      const res = await fetch(url)
      const data = await res.json()
      this.addRows(data)
      await sleep(1)
    }
  }

  @action
  addRows(data) {
    for (const row of data) {
      const item = new TodoItem()
      item.id = row.id
      list.items.push(item)
    }
  }
}

class TodoItem {
  @observable id

  constructor() {
    makeObservable(this)
  }
}

@observer
class TodoListEl extends React.Component {
  render() {
    console.log('re-render TodoListEl')
    return (
      <div style={{ wordWrap: 'break-word' }}>
        {list.items.map((i) => (
          <TodoItemEl key={i.id} item={i} />
        ))}
      </div>
    )
  }
}

function TodoItemEl(props) {
  return props.item.id + ' '
}

const list = new TodoList()

ReactDOM.render(
  <React.StrictMode>
    <TodoListEl />
  </React.StrictMode>,
  document.getElementById('root')
)

list.fetchPages()

[urugator]

How would you use flow here?

Flow is only effective if you have multiple mutations (like this.a = 1) directly in between awaits (rather than in seperate action, like you do with addRow). But you can use it anyway, it won't do any harm or difference, except it doesn't play very well with TS. Just replace async with * and await with yield:

@flow
*fetchPages() {
    for (let i = 1; i <= 40; i++) {
      const url = `https://jsonplaceholder.typicode.com/photos?_page=${i}&_limit=50`
      const res = yield fetch(url)
      const data = yield res.json()
      this.addRows(data)
      yield sleep(1)
    }
})

[hyperknot]

I see, so with flow I could get rid of the separate action, like this:

  @flow
  *fetchPages() {
    for (let i = 1; i <= 40; i++) {
      const url = `https://jsonplaceholder.typicode.com/photos?_page=${i}&_limit=50`
      const res = yield fetch(url)
      const data = yield res.json()
      for (const row of data) {
        const item = new TodoItem()
        item.id = row.id
        list.items.push(item)
      }
      yield sleep(1)
    }
  }

Interesting to know.

[hyperknot]

Thanks for all the help!