Closed savetheclocktower closed 6 months ago
If you do the stress test I described, you might also notice another quirk of the atom.ui.markdown
parser: it doesn't appear to handle HTML code blocks correctly in the preview pane, whereas the original parser does. That's a whole other ball of wax.
OK, this PR now wanders a bit outside of its scope in order to fix the HTML-encoding bug I mentioned above. I figured there was no point in putting it in a separate PR.
(bump)
…especially syntax highlighting.
The Problem
Suppose you’re editing a large Markdown document with lots of code blocks. Actually, don’t suppose — give it a try. Take one of my Tree-sitter blog posts and put it into a Pulsar editor in Markdown mode. Now copy everything but the front matter and paste it at the end to double its size; you should have about 50 code blocks at this point.
Now open a Markdown preview pane with Ctrl+Shift+M. Make any change at all in the buffer and notice how irritable you suddently feel:
This is a big enough annoyance to make me want to hide the preview.
The culprit here is the amount of time we spend doing things with editors. As you can imagine, it’s not exactly cheap to create a new
TextEditor
instance for each code block whenever we re-render a preview. But the real pain is in the inserting-into-the-DOM phase; we pay a large cost in style recalculation because a very complex element is newly present on the page. The editor component commits one of the textbook “sins” of style recalculation: when the custom element is first added to the page, it reads its ownoffsetWidth
andoffsetHeight
to determine whether it’s visible yet. That’s a “read” operation interleaved with a bunch of “write” operations — which the browser hates because it can no longer defer those writes until the most convenient time.Years ago, the Atom developers realized they could use lots of new CSS tricks in order to speed up rendering and minimize style recalculation. My instinct is that a lot of those tricks work fine when the editor exists in its “natural” environment (taking up the entire width and height of a workspace pane) but not at all when the editor is in an unusual environment (existing among other elements in a static layout). But that’s just a guess.
I don’t know of an easy way to fix the underlying issue with TextEditor, but I do know of a way that we can make it not matter.
As I mentioned earlier, the main problem with
markdown-preview
is that the entire contents of the preview pane are replaced whenever the content changes. You don’t notice it because most of the content is the same, but it’s still a lot of work being done for what are usually very small changes. And it means that if you have 50 code blocks on a page, each one of those code blocks (and theirTextEditor
s) is disposed of and recreated every time you type something — even something that’s outside of the code blocks altogether.Description of the Change
If Markdown were costlier to render, we’d have had to fix this ages ago. It isn’t, so this is a problem that’s only ever cropped up in documents with lots of editors for syntax highlighting. So the fix is this:
TextEditor
to represent its content instead of thepre
tag. But hide thepre
tag instead of removing it, because otherwise the diffing is a pain.TextEditor
instances and their customatom-text-editor
elements so that they don’t change during re-renders. The only cost we pay to create a new one is when a new code block appears on the page. And since thepre
element references themselves don’t change — because we’re diffing HTML, not replacing it — we can cache the editors using thepre
elements themselves as the keys.This is pretty straightforward. The strangest part of this is the fact that the
atom-text-editor
elements will be in the output, but we have to pretend they aren’t there during the diffing phase; if we didn’t, they’d get destroyed as part of diffing, since they’re not in the Markdown output that we’re trying to imitate. Luckily, morphdom — our diffing library — makes this pretty easy. (And it adds only 5K to our bundle size.)This strategy for minimizing the cost of syntax highlighting is useful enough that we’ll want to use it in both parser modes, which means that we’ll no longer be asking
atom.ui.markdown
to do our syntax highlighting for us. It’ll be worth it.Quantitative Performance Benefits
My test was to create the document I described above — one of my blog posts, but doubled. Then I showed a Markdown preview pane and made some changes to the source outside of a text block.
Here's a “before” snapshot from a profiler. A single character change resulted in 1.5 seconds of editor lockup:
On the other hand, the “after” snapshot captures me typing an entire sentence; each keystroke results in under 100ms of work. That's still a lot, but then this is a huge document, and the dropped frames are barely noticeable. The tooltip in this screenshot describes just one of these narrower bars on the timeline.
Possible Drawbacks
The upside of our previous strategy (emptying the element and starting over every time) is that it cleans the slate between renders. Now that we’re trying to persist stuff, new edge cases emerge related to rendering.
I definitely had to do some experiments to make sure that editors are being destroyed when they ought to be, are being recreated when they ought to be, have their contents updated when they ought to, and change from one language to another when appropriate. Luckily, the existing spec suite was thorough enough to tell me exactly where I’d been remiss, and all the tests now pass.
Verification Process
Mess around in that document I had you make at the top of this ticket. Stress-test it on both
master
and this branch. Do the things I mentioned in the previous section; switch ajs
code block to ats
code block, then switch it to atswtf
code block (a language string that maps to no grammar) just to make sure it turns back into an ordinary editor with no syntax highlighting.Applicable Issues
I pre-announced this PR in #893 in this comment that nobody really asked for.
Release Notes