kolodny / immutability-helper

mutate a copy of data without changing the original source
MIT License
5.17k stars 186 forks source link

Performance #83

Open deser opened 6 years ago

deser commented 6 years ago

Seems imutability-helper far away from Immutable.js when mutating. Are there any benchmarks or comparing imutability-helper with alternatives (seemless-immutable\timm\etc..)?

kolodny commented 6 years ago

I haven't really looked into it. I'd be open to a PR with a benchmark folder though

deser commented 6 years ago

Sounds great!

eldargab commented 6 years ago

Seems imutability-helper far away from Immutable.js when mutating.

Of course it is...

There are so many invariant checks and expensive paths, e.g.

    invariant(
      typeof spec === 'object' && spec !== null,
      'update(): You provided an invalid spec to update(). The spec and ' +
      'every included key path must be plain objects containing one of the ' +
      'following commands: %s.',
      Object.keys(commands).join(', ')
    );

JS engines are not yet that smart!

Otherwise, with right implementation, this approach should be faster then anything else for small and mid-size objects.

linq2js commented 6 years ago

here is benchmarks of common immutable packages https://github.com/linq2js/immhelper/blob/HEAD/benchmarks-result-03.txt

hoytech commented 6 years ago

I made a pull request to add update-immutable to the benchmarks. I copied the immutability-helper version of the benchmarks and just replaced it with update-immutable.

On my machine U-I is around 1.5-2x faster than I-H on most of the object tests. For arrays and the tests where it's bounded on deep-freeze it's basically a wash.

Immutable (immutability-helper)
  Verification: P-PPPP-PP-PPPPP-PP-PPPPPP-PPPPP-PPPP-PPPP-PPPP
  Object: read (x500000): 12 ms
  Object: write (x100000): 382 ms
  Object: deep read (x500000): 6 ms
  Object: deep write (x100000): 448 ms
  Object: very deep read (x500000): 34 ms
  Object: very deep write (x100000): 864 ms
  Object: merge (x100000): 221 ms
  Array: read (x500000): 6 ms
  Array: write (x100000): 21871 ms
  Array: deep read (x500000): 5 ms
  Array: deep write (x100000): 21658 ms
Total elapsed = 63 ms (read) + 45444 ms (write) = 45507 ms.

Immutable (update-immutable)
  Verification: P-PPPP-PP-PPPPP-PP-PPPPPP-PPPPP-PPPP-PPPP-PPPP
  Object: read (x500000): 10 ms
  Object: write (x100000): 125 ms
  Object: deep read (x500000): 6 ms
  Object: deep write (x100000): 202 ms
  Object: very deep read (x500000): 34 ms
  Object: very deep write (x100000): 438 ms
  Object: merge (x100000): 121 ms
  Array: read (x500000): 4 ms
  Array: write (x100000): 22116 ms
  Array: deep read (x500000): 6 ms
  Array: deep write (x100000): 22607 ms
Total elapsed = 60 ms (read) + 45609 ms (write) = 45669 ms.

And:

Immutable (immutability-helper) + deep freeze
  Verification: P-PPPP-PP-PPPPP-PP-PPPPPP-PPPPP-PPPP-PPPP-PPPP
  Object: read (x500000): 11 ms
  Object: write (x100000): 272 ms
  Object: deep read (x500000): 5 ms
  Object: deep write (x100000): 419 ms
  Object: very deep read (x500000): 31 ms
  Object: very deep write (x100000): 892 ms
  Object: merge (x100000): 236 ms
  Array: read (x500000): 5 ms
  Array: write (x100000): 22358 ms
  Array: deep read (x500000): 5 ms
  Array: deep write (x100000): 22344 ms
Total elapsed = 57 ms (read) + 46521 ms (write) = 46578 ms.

Immutable (update-immutable) + deep freeze
  Verification: P-PPPP-PP-PPPPP-PP-PPPPPP-PPPPP-PPPP-PPPP-PPPP
  Object: read (x500000): 11 ms
  Object: write (x100000): 116 ms
  Object: deep read (x500000): 6 ms
  Object: deep write (x100000): 186 ms
  Object: very deep read (x500000): 30 ms
  Object: very deep write (x100000): 416 ms
  Object: merge (x100000): 119 ms
  Array: read (x500000): 5 ms
  Array: write (x100000): 22935 ms
  Array: deep read (x500000): 5 ms
  Array: deep write (x100000): 23881 ms
Total elapsed = 59 ms (read) + 47653 ms (write) = 47712 ms.
linq2js commented 6 years ago

@hoytech That's great work, but I dont understand why you said UI 1.5-2x faster than IH, it seems UI slower than IH a little bit

hoytech commented 6 years ago

Am I misreading the benchmark?

IH: Object: write (x100000): 382 ms UI: Object: write (x100000): 125 ms

IH: Object: deep write (x100000): 448 ms UI: Object: deep write (x100000): 202 ms

IH: Object: merge (x100000): 221 ms UI: Object: merge (x100000): 121 ms

etc.

linq2js commented 6 years ago

@hoytech, i just compare total values haha

hoytech commented 6 years ago

Hehe yes I figured that's what you were looking at :)

I think the total is a bit misleading since it's heavily skewed to the longer running tests and the shorter tests get drowned out in the noise, however these tests seem like they may be the most important for many (most?) workloads.

That said, I haven't looked at how your benchmark is implemented in enough detail to know what it is actually testing for, so any extra information you have on that would be helpful.

linq2js commented 6 years ago

@hoytech you right I reviewed your source code and realize that we might get slow if update multiple branches of state tree for sample:

update(state, {
  a: {
    b: {
      c: {}
    },
    ...manyBProps
  },
  ...manyAProps
  }, { a: { b: { c: { $set: newValue }, $set: newValue  } } });

You use recursive call so it can process one spec at same time A and B props will be cloned twice so process gets slow down The performance test does not cover multiple update at once, that is big problem if app state should update many times

hoytech commented 6 years ago

Yes that is true: One of the advantages of the react update() system is that the overhead of the recursive traversal and shallow copying can be amortised over a number of simultaneous updates.

linq2js commented 6 years ago

What do you think about update() returning promise if there is any updated value is promise ?

hoytech commented 6 years ago

Hmm interesting. What do you see as being the use-case for this?

My concern is if this would break backwards-compatibility of the react update() interface.

linq2js commented 6 years ago
  1. for sample const nextState = await update(state, { todos: ['set', loadTodoFromServer()] }); if(nextState !== state) { this.setState({ ...nextState }); } does this make sense ?

  2. we can create new method updateAsync(); to explicit updating returns Promise obj and we also add more optiong like configure({ asyncUpdate: true }) to enable async mode for default update() func

hoytech commented 6 years ago

I think I understand, but just to clarify, what is the advantage over doing something like:

const nextTodo = await loadTodoFromServer();
this.setState(update(this.state, { todos: { '$set', nextTodo } }));
linq2js commented 6 years ago

Im implementing base class for covering update state using immhelper, something like

class BaseComponent {
  async update(specs) {
    const nextState = await update(state, specs);
    if(nextState !== state) { this.setState({ ...nextState }); }
  }  
}

// it easy to use rather than this.setState(update(...))

and action logic is clearly:
loadTodo() {
   this.update({ todos: Api.loadTodos() })
}
hoytech commented 6 years ago

Yes it's an interesting concept, although I'm wondering if there could be a way to implement it as a wrapper around update() instead of modifying its internals.

linq2js commented 6 years ago

Look at this https://codesandbox.io/s/0p22jvvoow This is my new package, it supports 5 ways to implement app