Page MenuHomePhabricator

Real Time Preview performance safeguards
Closed, ResolvedPublic5 Estimated Story Points

Description

We need to add some safeguards into Real Time Preview that prevent very long-running or rapid requests to the action=parse API.

Slack thread where the Editing team brought our attention to this:
https://wikimedia.slack.com/archives/C02CDL76HEH/p1645485371745649

Acceptance criteria:

  • Ensure there's no more than one in-flight request to the API at any time
    • If the wikitext is changed while a request is in flight, a final request should be made after the current one finishes to ensure the preview is up-to-date.
  • Average the response time from the server over say 3 requests, and if it is greater than say, 10 seconds (configurable), disable Real Time Preview
    • For now this can simply show an error message. Designs and switching to a 'manual refresh' workflow can be implemented later once we have the designs
  • Don't load Real Time Preview at all for very large pages (say 300K bytes) (we've decided not to do this since the previous safeguard will effectively cover this scenario as well)
  • These thresholds should have configuration settings so 3rd party wikis can control the behaviour based on how their production environment performs.

The thresholds can and may need to be adjusted after we test how Real Time Preview performs in a production environment such as Test Wikipedia.

Event Timeline

+ Performance-Team (might scale better than subscribing individuals)

Discussion from Slack:

esanders 22 Feb at 07:16
What are the performance considerations of this feature? Long articles can take tens of seconds to render. Triggering tens or hundreds of previews per edit could add significantly to CPU load, and many of those renders could end up being discarded by the time they arrive back to the client.
musikanimal
I don't think we've gotten that far, but from the top of my head we discussed automatically disabling the feature on very large articles.
The other idea is to load the full preview only the first time, then use data source range (DSR) info provided by Parsoid to only send the chunk of wikitext that has changed (i.e. the paragraph) to the action=parse API and update the corresponding part of the DOM in the preview output. The DSR approach still needs some investigation but it seemingly could work, with obvious exceptions such as a massive page of characters without any line breaks or whitespace. What do you think? (edited)
DavidL
Off the top of my head that'd hit issues with things like templates that open a <table>. (A.k.a. The β€œyou can't tell without parsing the wikitext whether it'll have far-reaching side effects” issue.) (edited)
esanders
And even balanced wikitext can have changes that affect other parts of the doc, for example removing a reference that is re-used elsewhere
esanders
Limiting the feature by wikitext byte size, or to section editing might prevent some of the worst cases, but you should probably have a conversation with performance and parsing teams.
samwilson
I've been wondering about this too. I seem to remember at some point we talked about the possibility of handling slower previews differently (which would also help people on slower connections), e.g. measuring how long it takes to fetch each preview, and if the time is increasing or is over some threshold, making the preview non-realtime (and add a button to trigger it).
esanders
Also once you've fired off a slow parse API request, it isn't really abortable. I think even if you abort the XHR request, the server is still going to run the parse to completion.
samwilson
that's true, but if we determine that one request is slow, should we avoid sending more?
esanders
yeah - I think if you are getting > X seconds for a round trip it would be reasonable to say the current conditions don't support real time preview (be that page size/network latency)
esanders
of course you can get outliers even on a small page with a good connection, so cutting off prematurely might be frustrating
samwilson
that'd possibly also mean that large simple pages would be fine, which would be nice.
samwilson
yeah, it'd want to be averaged over a few requests
esanders
you should also avoid aborting requests, and only fire off the Nth request when the (N-1)th has finished
esanders
so you don't queue up a dozen slow parses in parallel
samwilson
ah right, good point. the existing live preview should do the same, I'd guess?
esanders
(this is the opposite of how search bar autocomplete works, where queries are always cheap and minimising latency is the goal)
esanders
(we also don't do this with the live preview in the reply tool's source mode, but talk page comments are almost always tiny snippets of wikitext)
samwilson
so, if it makes a request and then waits for it to finish before sending the next, but the user has continued changing the page text, it might make for a bit of a lag in updating the preview. I wonder if some sort of visual indication might be needed to reassure the user that it's still working, because otherwise they'll see a response but it won't be the most up to date one and there'll be nothing to show that there's more to come.
esanders
yeah - if you are expecting slower updates a visual indicator would probably be useful
esanders
I think that is true regardless of your update strategy
musikanimal
sounds like we have a plan then: abort real time if it's slow-ish averaged over a few requests? that sounds good to me, and it'd be a lot easier to implement than the DSR-based 'partial previews'. I didn't think about templates or things that should change other parts of the page. Thanks for starting this conversation! We will be sure to run all of this by the Performance team, too.
musikanimal
This wishlist proposal was basically asking to bring en:User:TheDJ/ActualLivePreview.js into MediaWiki, which does full previews as we're currently doing. Even though it's slow at times, I think users learn what it can handle and use it only in cases where it does. But we of course should impose some safeguards.
samwilson
yes, and even in the non-realtime mode I think it'll still be useful (if it can be triggered manually), because it's still nice to have a preview next to where you're editing so you don't have to scroll up and down as with live preview.


Also, in the design meeting we had a few hours ago, we talked about this whole topic, and came up with:

  • a non-blocking loading indicator (e.g. blue snake bouncing border at the top of the preview pane) to show whenever loading is happening
  • match the debounce time to that of DiscussionTools preview (~2 seconds?)
  • time the requests and stop realtime previewing if they're taking more than ~10 seconds (threshold t.b.d.)
  • add a "this page is too slow for realtime preview" message, with manual button (akin to what's used for loading in Wikimedia OCR)
  • only request previews after previous requests have returned, and make sure to always show the loading indicator so users know that the state of the preview might be out of date
  • show the same error message as for Live Preview, in the preview pane
JJMC89 renamed this task from Performance exploration to real-time preview performance exploration.Feb 25 2022, 5:31 AM
MusikAnimal renamed this task from real-time preview performance exploration to Rreal-time preview performance safeguards.Mar 10 2022, 5:58 PM
MusikAnimal renamed this task from Rreal-time preview performance safeguards to Real Time Preview performance safeguards.
MusikAnimal updated the task description. (Show Details)
JMcLeod_WMF set the point value for this task to 5.Mar 10 2022, 6:21 PM

Have we thought about coupling parse requests with the action=stashedit requests? The latter is also "real-time" with a debounce of 3 seconds IIRC. Instead of sending the full wikitext twice, how about creating a single API endpoint that does parse + stashedit? Or maybe modify either of those APIs to add an option to also do the other operation?

Change 770640 had a related patch set uploaded (by MusikAnimal; author: MusikAnimal):

[mediawiki/extensions/WikiEditor@master] Real Time Preview: wait for a response before making new requests

https://gerrit.wikimedia.org/r/770640

Have we thought about coupling parse requests with the action=stashedit requests? The latter is also "real-time" with a debounce of 3 seconds IIRC. Instead of sending the full wikitext twice, how about creating a single API endpoint that does parse + stashedit? Or maybe modify either of those APIs to add an option to also do the other operation?

I haven't verified, but I believe action=parse will pull from the parser cache if an identical stashed edit is already present. But you make a good point that these APIs could do both. From a performance standpoint, I think it's ideal to use stashed edits, but from a product standpoint it may not work out since stashed edits are intended to pre-parse content under the assumption the user is about to save. Real Time Preview is different because it's meant to give you continual updates rather than doing so only when it thinks you're going to save. Additionally edits (currently) aren't stashed when creating a new section, and possibly other situations as well.

We were going to look into stashed edits when we move to using Parsoid, since it that's all the RESTBase API apparently accepts, so it's good to think about these things now. I'll run the idea by the team!

Change 770640 merged by jenkins-bot:

[mediawiki/extensions/WikiEditor@master] Realtime Preview: wait for a response before making new requests

https://gerrit.wikimedia.org/r/770640

Change 773354 had a related patch set uploaded (by MusikAnimal; author: MusikAnimal):

[mediawiki/extensions/WikiEditor@master] Realtime Preview: disable if response times avg over 10 secs

https://gerrit.wikimedia.org/r/773354

Change 773675 had a related patch set uploaded (by MusikAnimal; author: MusikAnimal):

[mediawiki/extensions/CodeMirror@master] Fixes for WikiEditor's realtime-preview feature

https://gerrit.wikimedia.org/r/773675

Change 773354 merged by jenkins-bot:

[mediawiki/extensions/WikiEditor@master] Realtime Preview: disable if average response time is very slow

https://gerrit.wikimedia.org/r/773354

The "average the response time" bit may be pretty challenging to QA. If you have a local dev environment set up, change $wgWikiEditorRealtimeDisableDuration (which is in milliseconds) to something really short, and that will make it easier. Otherwise you can use various tricks to simulate a slow internet connection, similar to what was suggested at T303383#7805296.

The values we're using now are what's in the task description: if the past three requests for preview average over 10 seconds, the error is shown.

@MusikAnimal or @Samwilson any ideas on how to test this. I'm thinking a functionality test of the real time preview is one way. Let me any other test I can perform in the front end. Thank you.

In addition to what @MusikAnimal says above, you can also test the timeouts by trying to preview a huge page (I find on my local environment previewing something big copied from Wikipedia works, especially if there are lots of images and InstantCommons is turned on). Also, adding a sleep(11) to ApiParse::execute() can help test a quick connection with a slow parse. The inverse can be done with browser tools for slowing the connection down.

With the counting of requests, remember that the first preview counts for one (i.e. if RTP is already enabled when the edit form is opened). So generally it should be a matter of seeing three long grey-out periods followed by the slow-connection warning, but it might only involve typing/pausing twice to make it happen.

NRodriguez added a subscriber: NRodriguez.

Amazing work!