Optimize post-remote processing about untangled-client HOT 17 CLOSED

For a 7.5 KB response in our app I saw the following

• om-plumbing/sweep-missing was called 3 times and took a total of 242 ms.
• om-plumbing/resolve-tempids was called twice and took a total of 109 ms.
• data-fetch/swap-data-states was called once and took 128 ms.
• om-plumbing/mark-missing was negligible performance-wise.

from untangled-client.

Based on these numbers it looks like the offline approach of om-plumbing/mark-missing is a fine way to do things. This offline algorithm, as I understand it, goes like:

• First annotate metadata based on the query.
• Then in the merge function you act according to those annotations.

We might be able to do something similar by annotating the response (perhaps rename mark-missing) for where the corresponding query has a non-join prop. Then the sweep-missing and resolve-tempids can be made shallow for anything with those annotations.

from untangled-client.

Right, so if you use something that walks the query/data you're trying to merge and marks the bits with metadata...actually, the existing mark-missing could be repurposed to add that metadata as it works, so that post-processing steps could leverage it (including tempid replacement AND deep-merge, in that order (since deep-merge will end up losing the metadata). That way we wouldn't need to repeat that processing.

You may find that a specter compiled transform is so fast that it doesn't matter to make that more complicated....it becomes a single line of code for tempid replacement. Simplicity is king and it would be good to measure something simple before replacing it with something complex.

Deep merge isn't all that complicated to begin with...so leveraging the metadata there is simple enough.

Changing the sweep step to a specter compiled transform (if it isn't already) would also be a potential win....and why is it being called 3x??? In fact, why are any of those being called more than once? That sounds like a bug to me.

The swap-data-states is another potential specter transform optimization

from untangled-client.

Just to be clear, do you think it's a bug (these functions being called multiple times) in untangled-client or my application?

from untangled-client.

I don't know. It strikes me as odd and probably a bug somewhere.

from untangled-client.

I used specter to optimize sweep-missing, resolve-tempids, and swap-data-states. I'm hoping that improves the post-merge performance problems. On 0.4.11-SNAPSHOT on clojuars.

from untangled-client.

Looks like I needed to fix untangled server's version of specter...the API imports changed and having both on classpath caused badness. 0.5.1-SNAPSHOT of server is on clojars.

from untangled-client.

On tempid remap: We need to walk everything here, because there is no telling where you embedded temp ids; however, added a short-circuit that avoids the walk altogether if you didn't create anything new with a tempid on the remote mutation. That should eliminate that overhead entirely for these large data set queries. I'm not sure how to make this much better...there is no way but an exhaustive search to make this consistent.

from untangled-client.

Set global loading optimized by using a top-level set of load UUIDs. Added these UUIDs to the markers, and callbacks were rewritten to maintain them properly (loaded and error). This eliminates the need for any kind of state walk. Also changed the callbacks to properly update the data states, so swap-data-states is now not needed at all.

from untangled-client.

Oh that sounds like a pretty good approach, can't wait to try it!

from untangled-client.

Just finished with the final optimization: modified the deep merge to take care of the sweep, and also stop at leaf nodes in the data (which were marked earlier by mark-missing). This required significant changes, so I added more tests for the merge, sweeping, and callbacks.

As far as I can tell, this should not break things. Testing it against todo mvc now.

from untangled-client.

Found one minor problem, fixed that, and now all of my tests look solid. Pushed up to 0.4.11-SNAPSHOT...waiting for external verifications/tests.

from untangled-client.

OK. All success on speedups for the targeted areas.

The forced re-render is the next target...I'll keep this ticket open.

I think we can probably leverage Om's follow-on read feature to fix rendering issues, and speed this stuff up.

from untangled-client.

Here's an example of om/force-root-render! causing a bottleneck in processing data that comes back, in this example, the API call returned and it took 1.03 seconds to force-root-render in impl.data-fetch/loaded-callback (in a case without post-mutation), and then om turned around and rerendered again in the next animation frame, from the looks of it.

from untangled-client.

Likely part of the reason for the exaggerated time of 1s here is that there are some complicated react components to render onto the page with this data set, but doing the render twice makes it much worse than it could be.

from untangled-client.

OK, forced root render has been reduced. I have some ideas on reducing it further (see TODO/FIXME tags in code). Basically, leverage follow-on reads (as we should have from the start).

This is up on 0.4.11-SNAPSHOT. Running apps through it to see about breakage.

from untangled-client.

Seems to work ok on our apps. We'll re-open if others see problems.

from untangled-client.