-
Notifications
You must be signed in to change notification settings - Fork 2.7k
MM-24781 Make GM profiles API calls to be concurrent #5436
Conversation
* Add p-queue as a dependdency and keep concurrency at 4 calls
|
||
await UserActions.loadProfilesForGM(); | ||
expect(UserActions.queue.onEmpty).toHaveBeenCalled(); | ||
expect(UserActions.queue.add).toHaveBeenCalled(); |
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.
i wanted to assert that we are calling an action here but it was becoming hard to mock the class etc.
@@ -37,6 +37,7 @@ | |||
"marked": "github:mattermost/marked#5a06d1af25ca48927b9efe0d9c42c01c84e7839b", | |||
"mattermost-redux": "github:mattermost/mattermost-redux#9a915443ab1d695540493406926c3dd269d380fc", | |||
"moment-timezone": "0.5.27", | |||
"p-queue": "^6.4.0", |
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.
The footprint of the library small and it has good API which I intend to use in upcoming features like prefetching posts. It has API's for clearing, pausing and adding tasks dynamically too which fits perfectly for some of the upcoming features.
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.
Awesome and a very clean way to make an impactful change 👏
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.
LGTM
added a minor change to keep the code cleaner
Co-authored-by: Guillermo Vayá <[email protected]>
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.
Thank you @sudheerDev
Tested, looks good to merge.
- Verified up to 4 requests are made at a time when there are many GMs on the sidebar.
Added test steps to the Jira ticket.
/update-branch |
Test server destroyed |
New commit detected. SpinWick will upgrade if the updated docker image is available. |
Test server creation failed. See the logs for more information. |
* MM-24781 Make GM profiles API calls to be concurrent * Add p-queue as a dependdency and keep concurrency at 4 calls * Update actions/user_actions.jsx Co-authored-by: Guillermo Vayá <[email protected]> Co-authored-by: Guillermo Vayá <[email protected]> Co-authored-by: mattermod <[email protected]>
* MM-24781 Make GM profiles API calls to be concurrent * Add p-queue as a dependdency and keep concurrency at 4 calls * Update actions/user_actions.jsx Co-authored-by: Guillermo Vayá <[email protected]> Co-authored-by: Guillermo Vayá <[email protected]> Co-authored-by: mattermod <[email protected]>
Summary
On the web, we now make one call at a time for GM profiles as attached in the screenshot. This can be improved by making 4 calls at a time in parallel and adding a new request to the stack when one finishes.
Making multiple calls will fetch all profiles sooner and instead of blocking the network stack with all the requests at once, we can cap them at 4. The reason to cap them at 4 is to have a little room for other network requests to go through in case they are needed while these are pending.
Ticket Link
https://mattermost.atlassian.net/browse/MM-24781
Screenshots
Before
![May-04-2020 19-03-10](https://user-images.githubusercontent.com/4973621/81010288-a8a52580-8e73-11ea-8a42-fa0669c64ac1.gif)
After
![May-05-2020 01-45-25](https://user-images.githubusercontent.com/4973621/81010225-8ca18400-8e73-11ea-9811-5490bc6b17e7.gif)