Skip to content
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

Speed up suggestion insertion by using a single transaction #11702

Merged
merged 1 commit into from
Apr 20, 2020

Conversation

mchowning
Copy link
Contributor

When adding @-mention support for Gutenberg, I noticed that it was taking a really long time for mentions to appear the first time for a site. Part of this is due to the API call taking 3-6 seconds when there are 1500+ users on the site (a site with only a single user responded in around 300ms).

Most of the delay was due to the time it took for the data to be inserted in the database one entry at a time. On my device, inserting 1500+ suggestions for a site took ~16 seconds after the API call returned the data. This PR wraps all those insertions into a single transaction which reduced the insertion time down to under 300 milliseconds in my tests.

This change is important for Gutenberg because we are going to be using this for a UI that is only used for providing suggestions. Having that UI not work for >20 seconds the first time it is loaded is obviously an awful user experience.

Testing

  1. Switch to a site you have not used on your device before (or wipe the app data).
  2. Open the comments for that site
  3. Tap on a comment
  4. Tap on the field for replying to a comment
  5. Note that the suggestions API call is initiated and that once it completes you almost immediately (< 300ms) see autocomplete suggestions when you type @ immediately followed by a single letter (until that point there won't be any suggestions available). Given how long the API call itself can take, I would suggest watching it using Stetho, Charles, or adding a log message to SuggestionService::handleSuggestionsUpdatedResponse so you know exactly when suggestions should be available.

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

On my device, when inserting 1500+ suggestions for a site, this
reduced the insertion time from ~16 seconds to <300 milliseconds.
@mchowning mchowning added this to the 14.7 milestone Apr 20, 2020
@peril-wordpress-mobile
Copy link

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link

You can test the changes on this Pull Request by downloading the APK here.

@mchowning mchowning mentioned this pull request Apr 20, 2020
3 tasks
@oguzkocer oguzkocer self-assigned this Apr 20, 2020
Copy link
Contributor

@oguzkocer oguzkocer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably should be doing this in a single query instead of a transaction. I think this is fine as a quick patch though, so :shipit:

P.S: It's very hard to look at this code for me, it's from before I switch to Android development 🙈

@oguzkocer oguzkocer merged commit ee116f1 into develop Apr 20, 2020
@oguzkocer oguzkocer deleted the speed_up_suggestion_insertion branch April 20, 2020 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants