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!
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.
(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.
@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!
(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/
(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!
(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.
(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.
Created attachment 463503 [details] Patch
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.
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.
(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.
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.
<rdar://problem/102504068>
Is it possible to try the patch for this now?
(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)
> 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
(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))
let's see what bug 249066 says about it.
(In reply to zalan from comment #18) > let's see what bug 249066 says about it. That one should be closed now?
We should just delete the complex text codepath entirely. *** This bug has been marked as a duplicate of bug 206208 ***