Raynos / observ-array

An array containing observable values
MIT License
26 stars 4 forks source link

implement `valueHasMutated` or `transact` #6

Open Raynos opened 10 years ago

Raynos commented 10 years ago

http://www.knockmeout.net/2012/04/knockoutjs-performance-gotcha.html

Having some mechanism to mutate the underlying array and then say list.valueHasMutated() or having an om/transact like construct

list.transaction(function (rawList) {
  // mutate raw list efficiently
})
// emits new shit after mutation

This is similar to thaw and freeze for mori

This seems incredibly difficult to do if it has any nested observables in the array because those have to be tracked and have changes bubble up to the parent.

For any array which contains only values and we want to push values in & out this should be possible.

Maybe we should have a observ-array/value-only function that creates a far simpler observable array with only values that can have efficient thaw and freeze.

mmckegg commented 9 years ago

I am currently thinking about implementing list.transaction. The easy way would be catching calls to set and store a list of the splice diffs, then call set for real at the end of the transaction.

Would most likely have to change the format of value._diff though. It could become an array of splice operations. Or preserve the current format, but the 'splice' would cover the full bounds of changes. The latter would work well where transactions were doing batch inserts, but this case is already covered by splice.

Thoughts?

Raynos commented 9 years ago

I also ran into the _diff cannot represent arbitary splices.

We can just bump major and make _diff an array of splice operations.

mmckegg commented 9 years ago

https://github.com/Raynos/observ-array/compare/transactions

Creates another instance of ObservArray, tracks all changes, then on transaction completion merges all the changes back into the original array then notifies the listeners.

I also switched _diff to be an array of splice operations.

What do you think of this approach?

Raynos commented 9 years ago

@mmckegg I think we want benchmarks

There are two options, pass a new observ array into the thing and then copy operations over.

Pass a fresh mutable plain array in then compare the array after mutating and apply diffs in one bulk.

The point of transaction() is

The thing that's really expensive is the shallow copies and emitting changes.

I think passing in an ObservArray will cause lots of unneeded shallow copies.

mmckegg commented 9 years ago

Great points.

The current implementation has solved the problem I was facing in observ-grid for now (currently just patched it in), but I will continue to look at better options. 

I agree some benchmarks would make things clearer. 

Will try calling with shallow copy of obs._list, then use adiff or something similar to generate the patches and apply them like now. 

Raynos commented 9 years ago

@mmckegg as an aside you can also see

^ those are really good reads about what I think the future of observ-* is

mmckegg commented 9 years ago

Just pushed my implementation of mutable raw lists with adiff.

https://github.com/Raynos/observ-array/compare/transactions

kristianmandrup commented 9 years ago

What is the status on this? I can see it was merged. How/what did it improve exactly? Thanks.

Raynos commented 9 years ago

transact is implemented & merged. Not sure how it can be improved.