Bug 247782

Summary:

Very slow DOM rendering of large (plain text/json) files

Product:

WebKit

Reporter:

Vladimir Prelovac <vprelovac>

Component:

Layout and Rendering

Assignee:

alan <zalan>

Status:

RESOLVED DUPLICATE

Severity:

Normal

CC:

beidson, bfulgham, koivisto, mmaxfield, rniwa, simon.fraser, webkit-bug-importer, zalan

Priority:

Keywords:

InRadar

Version:

Safari 16

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=248011

Bug Depends on:

248017, 249066, 248015, 248016

Bug Blocks:

Attachments:

Description	Flags
Patch	none
trunk vs. local changes	none

Vladimir Prelovac

Reported 2022-11-11 00:03:45 PST

Loading this URL https://raw.githubusercontent.com/json-iterator/test-data/master/large-file.json Takes unnecessary long time. On my system it takes about 2 seconds to load the file, and 28 seconds to render it in DOM, which is unexpected given it is a text file. Firefox does the same in about 12 seconds, which is still inefficient but still 2.5x faster. Even stranger, loading the file with pure JavaScript: fetch('https://raw.githubusercontent.com/json-iterator/test-data/master/large-file.json') .then((response) => response.text()) .then((data) => document.write("<pre>"+data+"</pre>"); Takes only 2 seconds, or 15 times faster than native browser loading!

Attachments
Patch (2.28 KB, patch) 2022-11-12 14:05 PST, alan	no flags	Details Formatted Diff Diff
trunk vs. local changes (21.84 MB, video/quicktime) 2022-11-13 14:28 PST, alan	no flags	Details
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Vladimir Prelovac

Comment 1 2022-11-11 10:54:52 PST

Even bigger JSON file is available for testing. https://raw.githubusercontent.com/zemirco/sf-city-lots-json/master/citylots.json In principle, Firefox handles loading/rendering much better but it feels like this file should be shown at the speed of loading and that vast amount time is lost somewhere rendering it the current way.

alan

Comment 2 2022-11-11 11:42:39 PST

(In reply to Vladimir Prelovac from comment #0) > Loading this URL > > https://raw.githubusercontent.com/json-iterator/test-data/master/large-file. > json > > Takes unnecessary long time. > > On my system it takes about 2 seconds to load the file, and 28 seconds to > render it in DOM, which is unexpected given it is a text file. > > Firefox does the same in about 12 seconds, which is still inefficient but > still 2.5x faster. > > Even stranger, loading the file with pure JavaScript: > > fetch('https://raw.githubusercontent.com/json-iterator/test-data/master/ > large-file.json') > .then((response) => response.text()) > .then((data) => document.write("<pre>"+data+"</pre>"); > > Takes only 2 seconds, or 15 times faster than native browser loading! It's curious. For some reason (easy to figure out, just haven't debugged it yet) the direct .json file rendering goes down the legacy line layout codepath, while the "document.write()" case exercises the modern line layout codepath (at this point most of the inline content should go through the modern codepath, so it's rather interesting as to why we fallback to legacy for case #1). Thank you for filing it.

Vladimir Prelovac

Comment 3 2022-11-11 12:21:25 PST

@zalan Thanks for looking into it! One more observation I made while testing is that document.write() was about 25% faster than document.body.innerHTML=() for this use case. Any thoughts on this and what is in principle the fastest way to get (large) content into DOM? Also can you easilly identify what kind of other content goes down the legacy rendering path, would be curious to test and provide more input!

alan

Comment 4 2022-11-11 12:33:45 PST

(In reply to Vladimir Prelovac from comment #3) > @zalan > > Thanks for looking into it! > > One more observation I made while testing is that document.write() was about > 25% faster than document.body.innerHTML=() for this use case. Any thoughts > on this and what is in principle the fastest way to get (large) content into > DOM? That sounds like a question to a DOM person. rniwa, do you have some insight on this? > > Also can you easilly identify what kind of other content goes down the > legacy rendering path, would be curious to test and provide more input! There's a visual indicator. It's a reddish glow (text-shadow) around the inline content when enabled through Safari's Debug menu (Debug -> Layout flags -> show legacy line layout coverage) https://www.droidwin.com/enable-safari-debug-menu-in-macos-monterey/

Vladimir Prelovac

Comment 5 2022-11-11 12:44:03 PST

(In reply to zalan from comment #4) > There's a visual indicator. It's a reddish glow (text-shadow) around the > inline content when enabled through Safari's Debug menu (Debug -> Layout > flags -> show legacy line layout coverage) This is interesting, thanks for sharing the tip. I can notice that a fairly large txt file gets rendered enterly through modern path (and fast) https://www.gutenberg.org/files/28054/28054-0.txt But legacy path is not exclusive to .json, as for example this also renders through modern: https://tools.learningcontainer.com/sample-json.json Hope that helps a bit!

alan

Comment 6 2022-11-11 12:55:47 PST

(In reply to Vladimir Prelovac from comment #5) > (In reply to zalan from comment #4) > > > There's a visual indicator. It's a reddish glow (text-shadow) around the > > inline content when enabled through Safari's Debug menu (Debug -> Layout > > flags -> show legacy line layout coverage) > > > This is interesting, thanks for sharing the tip. > > I can notice that a fairly large txt file gets rendered enterly through > modern path (and fast) > > https://www.gutenberg.org/files/28054/28054-0.txt > > But legacy path is not exclusive to .json, as for example this also renders > through modern: > > https://tools.learningcontainer.com/sample-json.json > > Hope that helps a bit! It does. Thank you for the examples! Will look into it shortly.

Ryosuke Niwa

Comment 7 2022-11-11 16:22:24 PST

(In reply to Vladimir Prelovac from comment #3) > @zalan > > One more observation I made while testing is that document.write() was about > 25% faster than document.body.innerHTML=() for this use case. Any thoughts > on this and what is in principle the fastest way to get (large) content into > DOM? That is not surprising given innerHTML setter would have to first construct a document fragment and then insert that document fragment whereas document.write will bypass that process and directly inserts content into the document.

alan

Comment 8 2022-11-12 14:05:52 PST

Created attachment 463503 [details] Patch

Vladimir Prelovac

Comment 9 2022-11-12 22:18:52 PST

Not sure if the patch is just intermediate step or I am using it wrong. I applied it to my local WebKit build and I still get the same behavior as before (slow rendering). The behavior continues also if I just "return false" in that function.

Antti Koivisto

Comment 10 2022-11-13 01:41:31 PST

Comment on attachment 463503 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=463503&action=review > Source/WebCore/layout/integration/inline/LayoutIntegrationLineLayout.cpp:178 > + return !isSegmentedTextContent(); I'm bit concerned that even pure text editing cases might be too slow without partial invalidation when there is a lot of text.

alan

Comment 11 2022-11-13 06:36:22 PST

(In reply to Antti Koivisto from comment #10) > Comment on attachment 463503 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=463503&action=review > > > Source/WebCore/layout/integration/inline/LayoutIntegrationLineLayout.cpp:178 > > + return !isSegmentedTextContent(); > > I'm bit concerned that even pure text editing cases might be too slow > without partial invalidation when there is a lot of text. Yeah, good point. Will make that change -though now thinking about it, maybe we should be even more strict on the editing case and bail out before reaching 128*65K text content.

alan

Comment 12 2022-11-13 14:28:09 PST

Created attachment 463508 [details] trunk vs. local changes Yeah, this patch itself does not address all the issues with large text content. If you don't mind, I am going to use this bug as an umbrella for the set of changes we need to make in order to speed up large <pre>text</pre> content handling - see attached screen recording.

Radar WebKit Bug Importer

Comment 13 2022-11-18 00:04:20 PST

<rdar://problem/102504068>

Vladimir Prelovac

Comment 14 2022-12-07 21:16:26 PST

Is it possible to try the patch for this now?

alan

Comment 15 2022-12-08 13:24:25 PST

(In reply to Vladimir Prelovac from comment #14) > Is it possible to try the patch for this now? yeah, one of the patches landed on trunk (the second one will be a bit delayed, that's a larger task)

Vladimir Prelovac

Comment 16 2022-12-08 16:35:45 PST

> yeah, one of the patches landed on trunk (the second one will be a bit > delayed, that's a larger task) I'e built from trunk and want to report that there is no observerable difference in behavior - it stil ltakes about 30 sec to render this https://raw.githubusercontent.com/json-iterator/test-data/master/large-file.json

alan

Comment 17 2022-12-09 20:39:58 PST

(In reply to Vladimir Prelovac from comment #16) > > yeah, one of the patches landed on trunk (the second one will be a bit > > delayed, that's a larger task) > > I'e built from trunk and want to report that there is no observerable > difference in behavior - it stil ltakes about 30 sec to render this > https://raw.githubusercontent.com/json-iterator/test-data/master/large-file. > json hah, it looks like any codepoint greater than HiraganaLetterSmallA throws us off of the fast codepath. -which is a shame as bug 248015 is a 3x speedup https://perf.webkit.org/v3/#/charts?since=1668203466687&paneList=((22-2050))

alan

Comment 18 2022-12-09 20:40:56 PST

let's see what bug 249066 says about it.

Vladimir Prelovac

Comment 19 2022-12-21 11:35:41 PST

(In reply to zalan from comment #18) > let's see what bug 249066 says about it. That one should be closed now?

Myles C. Maxfield

Comment 20 2023-09-22 21:50:30 PDT

We should just delete the complex text codepath entirely. *** This bug has been marked as a duplicate of bug 206208 ***

Note You need to log in before you can comment on or make changes to this bug.