-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
perf: change org performance improvement [INS-3968] #7582
Conversation
6e4455a
to
63869df
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added several minor comments.
} | ||
to={`/organization/${organization.id}`} | ||
}`} | ||
onClick={async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we should calculate route and async tasks in switching org, do we need to support CTRL+P
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems CTRL+P
already get the whole route. So we dont't need do it here.
taskPromiseList.push(syncOrganizations(sessionId, accountId)); | ||
} | ||
|
||
if (asyncTaskList.includes(AsyncTask.MigrateProjects)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just would like to double check, should we always check and migrate projects before SyncProjects
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
migrate projects only need to be execute in initial entry
101c7ee
to
c3bb78a
Compare
c3bb78a
to
63d822e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CurryYangxx initial tests I did show a 100 to 150 ms improvement from the click event until we draw another organization.
There's a couple of points I think could help the performance improvement:
- Whenever we switch between Orgs, we have some duplicate API requests being done:
- Reclicking on the exact same organization sends some requests which might not be necessary:
@filfreire Thanks for your test. Duplicate API requests are due to the remix's revalidate mechanism. I will start working on reducing the repeat requests next step. |
Changes
syncOrgsAndProjectsAction
after sync project.Spike
May be we can try to sync project in advance when app is idle