Simple observable collection optimizations

[TysonMN]

Fixes #134 and partially addresses #137 by implementing all of the "simple" optimizations.

I have not tested anything yet. I will test later.

The two remaining more complicated optimizations are removing all elements that need to be removed in nearly linear time (as discussed here) and ordering in nearly linear time (as discussed here).

[cmeeren]

Thanks a lot! Let me know when you're ready to merge. (PS, you can mark PRs as "drafts" or something like that when you create them, to signal if you don't want them merged right away.)

@reinux do you want to give this PR a spin to check how much it impacts the performance you mentioned in #113 (comment)?

[reinux]

Works great! Down from about 250ms per update to 120ms.

Haven't set up any proper tests, but it seems to work as expected.

The Seq.find is definitely the other big bottleneck at the moment.

Thanks, @bender2k14!

[cmeeren]

The Seq.find is definitely the other big bottleneck at the moment.

@reinux @bender2k14 Do you think the dictionary stuff in #137 (comment) could work well in this PR?

[reinux]

The Seq.find and the Move call that follows are actually what causes the bug that I mentioned in that comment. As @bender2k14 notes, caching the ID->ixs of the current items up front causes it to be out of sync as the Move calls change the indices. His branch actually uses the dictionary for the rest of the function, where that isn't an issue.

[TysonMN]

Indeed. Of all the planning that I did for this PR, I didn't notice that replacing Seq.fine with dictionary lookups would cause a bug. Good thing @reinux ran into it before me ;) :P

[TysonMN]

I added a commit that calls Clear when it would be efficient to do so

[cmeeren]

Seems that commit broke some tests. Could you investigate?

[TysonMN]

Oh, I had an improvement to that commit that I forgot to include. I force pushed a new commit. The tests all pass for me.

The previous commit had the correct behavior, but raised more (clear) events than it needed to. It is great to see that there are some tests that noticed the difference.

[TysonMN]

Thanks a lot

My pleasure :)

Let me know when you're ready to merge.

I am ready. I have reviewed my changes several times and have done some manual testing. I think everything is working.

PS, you can mark PRs as "drafts" or something like that when you create them, to signal if you don't want them merged right away.

Oh, great. I will keep that in mind for next time.

[cmeeren]

In #134 (comment) you mention adding a test; should that be part of this PR or should I go ahead and merge? (If it's related to the changes in this PR, it should ideally be part of it.)

[TysonMN]

It makes sense to add the test to this PR. Coming soon!

[TysonMN]

It took me longer than I expected to write the test. The good news though is that I think it is complete. I have it failing on master. Tomorrow I will confirm that it passes on the branch in this PR. Then I will force push the branch with the test created first.

[TysonMN]

Done! :D

Ready to merge

[cmeeren]

Thanks a lot! Will try to review and merge tomorrow.