memoize-state icon indicating copy to clipboard operation
memoize-state copied to clipboard

Fight the Spread

Open theKashey opened this issue 7 years ago • 2 comments

As continue of #5

Full object spread is a big problem - from point of view it is a performance killer, as long all object fields are set to be trackable, from another - this is why we emit a warning asking a user to reconsider selector, and that is not nice.

First version of "spread detection" just disables tracking during massive spread operation. Now - only record a fact, as long we have to emit a new "result" if any of fields got updated.

But how memoize-state tracks the keys?

  • "By use". there is a proxy get hook, to trap any access to a property. This is how we know about something got accessed.
  • "By result". it analyse the function result in search of values just passed through, then memoize-state have to deproxify result, replacing Proxies by original values. This is but sorting array input does nothing - it will be replaced by the original object, as long it represents it.

Also

  • there is "magic" field, "scan(Start|End)Guard", added to each object to detect spread.

Missing

  • we could add get/apply trap on some array methods( https://github.com/sorodrigo/on-change/blob/caa29f63ad78108a9006eeb82774088da80822bb/index.js#L35) to detect "spreading" of arrays, and sorting immutable input data.

The idea:

What if add a special flag, to deproxify result outside of function call? Just split the calculation and the data you are going to pass throught.

Also we have to keep in mind - some spreads, are 100% "legal", as long "things just works that way". For example <Element {...props} /> - all the props are passed down to nested component.

Approach number 1

So "by result" will track the keys from "result" to be replaced by real values from "input". We still could have function memoized, but keeping "spreaded" paths updated.

const fn = state => memoize({...state, value: state.a + state.b});
// "returns" deproxifyed result {...state, value}
// should react only to .a or .b changes. But react on all props, recalculating everything.

const fn = state => memoize({...state, value: state.a + state.b}, {lazy: true});
// "returns", "raw" result {...Proxy(state), value)
// Will react only to .a or .b changes. "unwapping" proxies to the "current" values on each call.

As long object Objects and Arrays got proxies - not work for simple states. Meanwhile - could handle almost all the cases.

Approach number 2

Create a special function like Object.assign, to perform this operation without extra magic. "scanGuards" is the magic, I'll better keep of, but without them, we could not distinguish when values were "used", and when just "returned"

cons fn = state => ({...state, a: state.a, b:state.a+1});
fn({a:1, b:2});

spreaded, returned(used), used. If value was spreaded or returned - it still could be used. Only b should be omitted.

Approach number 3

const fn = state => memoizedFlow(state, state => ({value: state.a + state.b});
// will produce "expected" behavior, as long it has `state = ({...state, fn(state)})` internally, outside of memozation tracking.

But, as long the goal of this library is to be "magic" - we might things about a better way.

theKashey avatar Apr 19 '18 22:04 theKashey

The spread detection is triggered in a case I don't understand

I have an app state with the following part :

{
  referentials: {
    clubs: {
      isLoading: false,
      ids: [
        {
          id: '/resamania/clubs/1',
          schema: 'Club'
        },
        {
          id: '/resamania/clubs/2',
          schema: 'Club'
        }
      ]
    },
    activities: {
      isLoading: true,
      ids: []
    },
    studios: {
      isLoading: true,
      ids: []
    },
    coaches: {
      isLoading: true,
      ids: []
    }
  }
}

And a selector like this

/**
 * Returns the current state of referential loading
 *
 * @param {object} state
 * @returns {boolean}
 */
export const getIsLoadingSomething = memoize(state =>
  Boolean(Object.values(getReferentials(state)).find(value => value.isLoading))
);

I'm getting

memoize-state: object spread detected in state => Boolean(Object.values(getReferentials(state)).find(value => value.isLoading)) . Keys affected: [".referentials"] . This is no-op.


What I understand is that I'm iterating over all referential state and tho I'm killing optimization, but I don't know how I can do things differently. I don't want to hardcode the list of referentials. This is killing the magic of memoize-state a bit!

I read this https://github.com/theKashey/memoize-state/issues/5#issuecomment-372809558 but still don't understand

alex-pex avatar Aug 24 '18 13:08 alex-pex

To say the truth - the current behaviour of spread detection is wrong - Full spreads or full enumerations are absolutely legal operations.

I am looking forward to disabling it.

theKashey avatar Aug 24 '18 23:08 theKashey