Speed up Home initialization #2259
Conversation
Old file now unused moved in cozy-dataproxy-lib.
The apps are already fetched at the root component and checked to determine if the app is initialized. There is no need to have additional custom logic pass down to children component.
When querying the triggers on the `worker` field, it uses a stack route that will complete the trigger information with jobs. This is costful, as it makes additional database requests, and useless at startup time. So, we now uses a `partialIndex` that will uses the `/find` route and make a faster query. See https://github.com/cozy/cozy-client/blob/c6762bb0c1fad4ac62f18b4118c8577a749caf2b/packages/cozy-stack-client/src/TriggerCollection.js#L125 and https://github.com/cozy/cozy-stack/blob/master/docs/jobs.md#post-jobstriggers
We get rid of the complicated state management to determine whether or not the Home is initialized, to rely on a dedicated hook in charge of fetching all the required data. Note the Home is relying on this mechanism to later fetch data directly from the store. In the future, we might have the same logic, but relying on the dataproxy directly
BundleMonFiles updated (7)
Unchanged files (6)
Total files change -374B -0.01% Final result: ✅ View report in BundleMon website ➡️ |
| @@ -18,7 +18,7 @@ export const makeTriggersQuery = { | |||
| definition: () => { | |||
| return Q('io.cozy.triggers') | |||
| .partialIndex({ | |||
| worker: 'konnector' | |||
| worker: { $in: ['client', 'konnector'] } // client is for CLISK | |||
There was a problem hiding this comment.
Sadly, as you explain in the description of this commit, this request does not fetch jobs. And these jobs are needed to compute the current_state attribute of the trigger which is needed to know if the trigger has any error.
I must admit I have some difficulties to follow the flow of the front code to know where and how it is used but if I try your branch in my local cozy, I don't have any error status displayed on konnector icons in home application.
There was a problem hiding this comment.
sigh looks like I missed this case. I guess we could query the triggers with job info at the moment we click on the konnector "group" as it is at his moment that we display the error icon, right?
| log('info', `Required data fetched in ${endTime - startTime.current} ms`) | ||
| loggedTime.current = true | ||
| } | ||
| }, [allQueries]) |
There was a problem hiding this comment.
You want to keep this monitoring in production ?
There was a problem hiding this comment.
Yes, because it might be later useful, and I don't see a strong problem with it, as the impact will be negligible. But I'm open to contradiction if you think this is bad practice
There was a problem hiding this comment.
It was just to be sure it was not a miss.
Also, I'm sure there are ways to monitor these kind of things with Sentry. But there's a time for everything.
There was a problem hiding this comment.
Yes a sailor told me that before he left :)
Let's keep it like this for now, it seems reasonable and might evolve in the future
|
Well done ! |
We used to query triggers with their `current_state` at Home startup. We removed it for performances purposes, but now it is only made when the focus event is fired. This means we can miss this state when entering a konnector group, which is required to correctly display the icons. So, we query the triggers when required, and mutualize it with the focus queries to benefit from the redux store.
paultranvan
force-pushed
the
improve-start-perfs
branch
from
48e75e1 to
80d945e
Compare
This is meant to speed up to Home initialization and thus its display, particularly for the flagship app.
Measured time:
[Web]
[React-Native]
For both environments, this is roughly a 4x in performances.