Improve performance when dealing with large json content #252
Comments
|
@j2jensen Have you checked the Wiki? There's a section named Handle Large Arrays with the JSON editor |
|
I have looked at the Wiki a few times. I'd never noticed that page, but I took some time to look it over. The link to the class it mentions is broken, and to be honest it doesn't really make sense to me. Have you read through it? |
|
@j2jensen Try here: https://github.com/niebert/Editor4JSON and here: https://github.com/niebert/json-editor-dorn |
|
Thanks for those links. I'll take a look!
…On Sat, Dec 22, 2018, 1:00 PM Peter Klein ***@***.*** wrote:
@j2jensen <https://github.com/j2jensen> Try here:
https://github.com/niebert/Editor4JSON and here:
https://github.com/niebert/json-editor-dorn
I know that @niebert <https://github.com/niebert> is a very competent
contributor/user of JSON-Editor. So I have no doubt that it is working.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#252 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAbrdzlZ7RCXNkqBgzMpXVdMxjY6sjOTks5u7o9ogaJpZM4ZdA-s>
.
|
|
@j2jensen Note: The fork of JSON-Editor at @niebert, is a fork of the original jdorn version of JSON-Editor. I have updated the links on the Wiki page. BTW: Do you have Danish ancestors? As your surname is a typical Danish surname. |
|
Yes, my great great great grandfather was a Danish painter who immigrated
to the United States. I have lots of Jens Andersens and Anders
Christiansens and such in my family tree.
…On Sat, Dec 22, 2018, 2:03 PM Peter Klein ***@***.*** wrote:
@j2jensen <https://github.com/j2jensen> Note: The fork of JSON-Editor at
@niebert <https://github.com/niebert>, is a fork of the original jdorn
version of JSON-Editor.
That version lacks several of the new features available in this fork.
BTW: Do you have Danish ancestors? As your surname is a typical Danish
surname.
(Im from Denmark myself, but people expect me to be German due to my
surname. 🤣 )
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#252 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAbrd8D9XZ2R0oNrR9hmtYN1Fo6KoY6bks5u7p4mgaJpZM4ZdA-s>
.
|
|
All right, I was able to take a look at what he's doing, and I understand it. It's not quite the same problem we're running into, because instead of having hundreds of simple items in one big top-level array, we've got a complex nested structure. There's at most a dozen elements in any given array, but it adds up to hundreds of overall array elements and thousands of nodes once you traverse the entire tree. If necessary, we could probably rearrange our schema so that the top-level properties can be edited as part of one object and the "filter grains" can be selected from a list and edited independently. I think any individual grain would probably be small enough that it could be loaded in an editor without timing out the browser. But that feels a bit like a work-around for something that the editor itself seems well-situated to handle by itself. If you use the schema and json that I've posted in the interactive demo, you can see that I'm using tab formatting for various array types (grains, columns, filters, and filter components), so most of the UI elements that get generated never actually get rendered in the browser. If those tabs' contents could be generated on-demand the first time the tab is opened rather than all getting generated when the editor is first initialized, it would be vastly faster. Of course, I haven't dug through the json-editor code enough to understand whether this would actually be anywhere near as simple as it sounds. I know things like this are never as easy as they seem, and I don't want to be that guy that says, "Can't you just...?" But I thought I'd mention that the time taken to generate all the DOM is impacting us, and if there's any way to solve that within the editor itself that would be fantastic. |
|
@j2jensen The tabs/categories are handled by the object editor (editors/object.js) and it currently has some unresolved issues (see #214) I think building the form dynamically upon expanding would require major changes, but you can create/extend your own custom object editor if you feel up for the task. 😉 |
|
I checked out that PR's code, and it doesn't seem to make much of a difference, which doesn't surprise me. Profiling performance, it appears that 25-30% of our time is spent in the The most likely source for trouble in this method is
We could spend some time digging into some of these activities to see if their speed can be improved, but even if we got all their time down to practically zero, that only accounts for about half of the overall time spent. And to be honest, there's probably not much we can do about things like |
|
We could probably improve the speed of DOM creation by using document fragment instead of applying directly to DOM. As fragment does not create reflows (repainting the page) when you add/delete a node. Currently, each editor manipulates the DOM to insert/remove the HTML code. So to be effective, we will have to change that so they only manipulate the fragment. And then when all editors are done rendering the HTML, insert the fragment into the DOM. Here are some jsperf tests that show the speed difference: |
|
Are you specifically talking about using document fragments in If the former, then I don't think that's really necessary. Since we're creating a text node to add to the button anyway, I'm pretty sure something like this would be simpler and faster than using a document fragment: If the latter, then it probably depends. I get the impression that working with a document fragment is pretty similar to working with any element that hasn't been added to the document yet. The last link you provided, for example, shows that using a document fragment is actually slower if you were just building DOM elements anyway. I've been working under the assumption that the JSON Editor DOM is all built in-memory and only added to the document once everything is built. If that's not the case then we probably could see a significant performance boost by doing so, regardless of whether we use documentFragment. Regarding the original approach I'd suggested, I notice that when we use |
Yes. Doing simple stuff like the last test example, won't help speed up the rendering. Only if all editors push elements to fragment and then at the end the fragment is inserted. But I'm not sure if it is possible without a major rewrite. Using innerHTML could be one of the culprits of the slow DOM creation. So maybe we should look at changing all occurrences of that? InnerHTML Test
Each editor inserts code when called. Some editors (like Array and Object), triggers the editor when you click the button/select box. |
Even this wouldn't actually have much of an impact. As this test shows, adding elements to a DOM element which is detached from the document body is every bit as fast as using an HTML Fragment. Since that's largely what the editor does already, using HTML fragments won't change much. So that approach is a red herring. Likewise, since innerHTML is vastly slower than other approaches, it's easy to suppose that we can improve performance a lot by not using it. But the InnerHTML Test you linked shows that even though it's roughly 40x as slow as the alternative, it can still run 100,000 times in 100 milliseconds. And a perf test using chrome's dev tools doesn't appear to indicate that we're spending lots of time in innerHTML.
Looking at what's actually causing poor performance, the single biggest issue that jumps out to me is that we're taking the time to build out the entire DOM before allowing user interaction, even though (in a schema like mine) only 2% of it is even going to be seen. Addressing any of the other issues is likely to shave off maybe a hundred milliseconds here or there, but if we simply limit our focus to creating the elements that actually get rendered, we'll make the user experience practically instantaneous. |
|
I've been tinkering with the code, and I think I see a good way forward. Here's what I'm thinking: When the Array editor is initializing its rows, if it's set up to use tabs it can keep track of what the row's value should be, without actually building out that row; we'll say that this row is "deferred".
This makes the initial load much faster. Setting up the editor is no longer the bottleneck: now validation is the biggest performance bottleneck. However, since each tab's contents are initialized once it's selected, and since setting an editor's value causes validation to get kicked off, that validation cost is incurred each time a new tab is selected. To mitigate this, I'm planning to change core.js to cache the most recently-validated json and only call The only snag that I've identified in this plan is that the tab headers are currently populated by asking the row's editor for its header text. If we haven't initialized the editor yet, then we can't expect to get the text that way. I imagine I'll find a way to make this work, but I haven't yet and I'd be open to any suggestions. I would also love any other input or warnings you'd have about this approach. Are there corner cases I need to watch out for that I might not be thinking about? Would you accept a pull request that takes this approach? Should this deferred loading of tab contents be some kind of configurable option? If so, should it be configured editor-wide or in the schema itself? Should it be enabled or disabled by default? |
|
There are a couple things you might want to test.
If you can implement this new feature so setup/usage is identical to before, I see no reason to add a config option. Only if the setup/usage differs there's a need to be able to turn the feature on/off. |
|
Hey! If I'm not clear enough, tell me and I'll try to explain myself better ;) Thanks, |
|
I just updated the links to the CDN files, So if you have been using CDN links in your tests, you need to update the link to the new format. (See the docs page) |
|
@Fluf22 thanks for adding to the discussion. I've pushed my first stab at this to a branch on my fork: https://github.com/j2jensen/json-editor/tree/performance-improvements My current approach doesn't load the headers correctly. I'd like to see your changes and compare notes. I haven't had a chance to check how my branch behaves with watches. Supposing it interferes with watch behavior, though, that's probably something that should involve a little discussion here. It seems like we might be able to make watches work by handling them at a high level and using the current editor paths to figure out which handlers to invoke when an event is fired (kind of like jQuery's Alternatively, we could make the deferred loading configurable and opt-in (either editor-wide or as an option on specific schema elements), and add documentation stating this is a good way to improve loading performance but it'll prevent watches from working on array elements. I have a deadline coming up for some unrelated work so I probably can't spend much time on this for the next few weeks, but I would like to see a resolution to this maybe some time in February. |
|
@j2jensen I looked at your code (but didn't test it), and asked myself a question. When I tried to improve the performance on my side, I stumbled upon some edge cases, like value refresh when adding / deleting / copying / moving items. Don't you had the same issues ? Because your code is much more clearer than mine! Thanks, |
|
@Fluf22 Since I've only had a few hours to work on this, I haven't had a chance to run into those edge cases yet. Playing with my solution now, it does look like there's some weirdness when adding an item, but copying, moving, and deleting all seem to work fine. When I have time later in the month I'll look into a way to fix adding an item, as well as looking over your changes. I would love to hear about any other edge cases that you found to be problematic, to make sure we address those. |
|
@j2jensen Okay, let me know when you have time to work on it. |
|
Update: we've discovered that in our case, even though the editor does take a long time to load (10+ seconds), what was actually making it crash the browser for some users was when they had the LastPass extension installed. I'm not sure if LastPass just started taking a long time to scan the fields to try to determine if they're for a form, or if we hit on some corner case that threw it into an infinite loop, or what. I've logged an issue with LastPass to see if there's any way to simply prevent it from trying to scan our site in the first place. In the meantime, we'll just have to make sure the handful of users that access the page where we use this editor are instructed to disable the extension for our site. Knowing that the real issue for us was more related to that plugin than to the editor's actual performance, there's a chance that this will not take priority for me, although I would like to pick it up again if I can find any "spare time". |
|
@j2jensen I know it's probably a long shot, but did you ever get any resolution with LastPass? We encountered the same thing, those with LastPass installed see SIGNIFICANT delays (like 4-5x)! |
|
No real resolution, no. They pointed me to this FAQ, which I was already aware of: https://lastpass.com/support.php?cmd=showfaq&id=10512
Thing is, we're not trying to prevent them from adding their lastpass icons: we're trying to prevent them from scanning the DOM in the first place. I haven't tested adding this attribute to all the text boxes, but I doubt it'll address the problem. It wouldn't hurt to have more people log issues with LastPass. Maybe it'll draw more attention to the issue for them. |
|
I'm not familiar with Lastpass myself, but from the link @j2jensen posted, it looks like it inserts some kind of icon for each form field. It that's the case, the |
@Fluf22 It would be nice if we could make a speed comparison test between the main branch and yours. Or have you made some tests yourself? |
|
Actually, I work with very large JSON objects everyday, but my company won't let me give them to you as tests 😀 |
|
Can you use the schema/json I posted and just duplicate the top-level objects until you get to an object that's the size you need? For my part, my team has plans to start breaking down the objects we're dealing with into smaller pieces anyway, for reasons other than the performance of this editor. So it probably won't be a high priority for me to engage with it. However, I'll have some time off on the first week of July so I might jump into it for a bit then for fun if it looks like we might have a promising path forward. |
|
@pmk65 Not yet, sadly! I didn't get the time, but I may be able to dive into it again, soon. Perhaps mid-july. |
|
Do any of the ideas from https://stackoverflow.com/questions/20954944/stop-lastpass-filling-out-a-form work? |
This comment has been minimized.
This comment has been minimized.
|
@milahu Done. Thanks for teaching me about that. |
Expected behavior
People want to be able to drill down through a large json document to make changes to a few specific parts of it.
Actual behavior
Loading a large json document causes the browser to do a lot of processing. On slower machines, this can even cause the browser to warn the user that the page is unresponsive and prompt them to kill the tab.
Profiling appears to indicate that most of the CPU time is spent building out DOM elements, most of which are never seen because they're collapsed or on tabs that don't get accessed.
It seems to me that by lazily constructing collapsed elements the UI could load pretty much immediately, and the portions that need to be built when the user selects a particular tab or expands a particular collapsed object could usually be built quickly enough to go unnoticed by the user.
Or if there's another way to optimize the construction of the DOM elements to improve performance by orders of magnitude, that would work too.
Steps to reproduce the behavior
Schema
JSON Schema
JSON
Sample JSON Content
The text was updated successfully, but these errors were encountered: