immerjs / immer

Create the next immutable state by mutating the current one
https://immerjs.github.io/immer/
MIT License
27.65k stars 848 forks source link

Faster iteration experiment #1120

Open mweststrate opened 5 months ago

mweststrate commented 5 months ago

Object.keys is faster than Reflect.keys. Can we leverage this somehow? (at the cost of symbol props etc) https://www.measurethat.net/Benchmarks/Show/18717/0/reflectownkeys-vs-objectkeys-preformence#latest_results_block

markerikson commented 5 months ago

FWIW, I've actually been doing some RTK perf optimizations over in https://github.com/reduxjs/redux-toolkit/pull/4361#issuecomment-2071038122 , and found that 1) it was faster to do some reads on a const ids = current(state.ids) array instead of a wrapped array, 2) the majority of the remaining overhead seems to be in Immer, 3) a lot of it looks like it's in finalizeProperties

mweststrate commented 5 months ago

See also https://github.com/immerjs/immer/pull/1107. There are several ways to make Immer quite a bit faster, but it would required much more loosely treating things that have getters/ non-enumerables / symbolic properties, which would benefit the majority of the user base, but also create enough failure scenarios to flood the issue tracker😅

markerikson commented 5 months ago

@mweststrate out of curiosity, how much of a perf difference do you think this will make?

FWIW it looks like only one test failed:

FAIL __tests__/base.js
  ● Test suite failed to run

    TypeError: Cannot perform 'get' on a proxy that has been revoked

      2379 |        })
      2380 |
    > 2381 |        expect(state[sym].x).toBe(2)
           |                          ^
      2382 |    })
      2383 | }
      2384 |

      at __tests__/base.js:2381:21
      at testObjectTypes (__tests__/base.js:2368:2)
      at __tests__/base.js:1861:28
      at __tests__/base.js:1860:3
      at runBaseTest (__tests__/base.js:61:2)
      at Object.<anonymous> (__tests__/base.js:32:4)
mweststrate commented 1 month ago

@Pyaepyae-art please stop randomly approving draft or merged PRs. I'm not sure why you are doing this. I hope it is unintentional. But just in case, on the next contextless review you will be banned from the repo.