concat and observable.array

[logrox]

When in the observed array I have few elements and do "concat" below, everything is as expected, but if it will be more data, eg 60,000 elements, then the problem begins.
See for yourself.

my code

[danielkcz]

Thanks for the report. It might be a performance problem related to mobxjs/mobx-react#877.

Note that MobX is not built to be performance winner, there are better tools if you need to handle big amounts of data.

@urugator just FYI since you are rewriting array handling for V6.

[urugator]

Can't "debug" it because the fiddle is randomly freezing even without observable array.

[danielkcz]

https://codesandbox.io/s/affectionate-wildflower-x5kqu?file=/src/index.js

CodeSandbox is at least clever enough to spot a problem and doesn't freeze.

That's a different problem, but I suppose you can debug with lower number of iterations.

[mweststrate]

This might be hitting the maximum function arguments somewhere, given the 60.000 number. https://stackoverflow.com/a/22747272/1983583. If we ... or .apply an array function in our concat implementation that could explain.

[mweststrate]

as an work around observableArray.slice().concat(otherObservableArray.slice()) will probably work.

[urugator]

Ah I think there was a misunderstanding this isn't about performance...
The problem is [1,2].concat(hugeObservableArray) results in [1, 2, undefined, undefined, undefined, ...]

[urugator]

it misbehaves on observbaleArray.length >= const MAX_SPLICE_SIZE = 10000

[logrox]

Ah I think there was a misunderstanding this isn't about performance...
The problem is [1,2].concat(hugeObservableArray) results in [1, 2, undefined, undefined, undefined, ...]

You're right, I meant undefined

[mweststrate]

...aahh I thought it was about the app hanging (at least for me in chrome). Urugator are you interested in creating a unit test for this on current master branch?

…

On Fri, Jun 19, 2020 at 12:04 PM logrox ***@***.***> wrote: Ah I think there was a misunderstanding this isn't about performance... The problem is [1,2].concat(hugeObservableArray) results in [1, 2, undefined, undefined, undefined, ...] You're right, I meant undefined — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2379 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN4NBEMP6KD54VJ7VPH4FDRXNA3ZANCNFSM4OCNP3LQ> .

[urugator]

Current master meaning v6? I could, but note some test are already failing there. I can also add it to #2373 If you want. Btw is this even fixable? It seems to be related to that MAX_SPLICE_SIZE, but notice it's concat method of the native array (not our concat). I tried [1,2,...hugeObservableArray] but that seems to work fine, so I wonder what that native concat really does with our array?

[mweststrate]

Not sure, but having a regression test will already be valuable. Sorry for being slow in follow up lasts weeks, with children at home full time I find it particular hard to find time and focus, so efforts are a bit random at the moment 😊 Op vr 19 jun. 2020 12:28 schreef urugator <[email protected]>:

…

Current master meaning v6? I could, but note some test are already failing there. I can also add it to #2373 <#2373> If you want. Btw is this even fixable? It seems to be related to that MAX_SPLICE_SIZE, but notice it's concat method of the native array (not our concat). I tried [1,2,...hugeObservableArray] but that seems to work fine, so I wonder what that native concat really does...? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2379 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN4NBDPWWXRWPEYCJDWFM3RXNDVRANCNFSM4OCNP3LQ> .

[urugator]

I've added the test to #2373, so feel free to merge it.
The result is different for legacy array because it's concat(object) rather than concat(proxiedArray)

[mweststrate]

awesome, thanks!

…

On Fri, Jun 19, 2020 at 5:27 PM urugator ***@***.***> wrote: I've added the test to #2373 <#2373>, so feel free to merge it. The result is different for legacy array because it's concat(object) rather than concat(proxiedArray) — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2379 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN4NBGEHEP4JZVS3B3WI3LRXOGYHANCNFSM4OCNP3LQ> .

[logrox]

But when I use the na.concat(oa.toJS()) everything works as it should.