Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance #16

Closed
shornuk opened this issue May 18, 2016 · 15 comments
Closed

Performance #16

shornuk opened this issue May 18, 2016 · 15 comments
Assignees
Milestone

Comments

@shornuk
Copy link

shornuk commented May 18, 2016

Hi Benjamin.

I'm just wondering if you've hit many performance issues with Neo yet? I have a pretty large Neo field that includes an awful lot of fields including quite a few Matrix fields. I find saving a page with the Neo field is quite slow. I've had a couple of errors when my php memory was set low at 8M, but it's been upped now and although slow, I've not had any errors. I haven't tested properly yet and I'm sure there's some optimisations I can make, just wondering if you've hit anything yet?

@benjamminf
Copy link
Contributor

benjamminf commented May 19, 2016

Hey Sean, yes it's something I've encountered. I know of one particular bottleneck, which is actually the rendering of the field. Large Neo fields can render hundreds of kilobytes of raw HTML/JS, which will probably be a strain on the server. A lot of the generated HTML/JS doesn't change, so caching it shouldn't be a problem.

Another potential bottleneck, which I haven't verified yet, is the DB insert queries. After every save Neo field structures are completely torn down and rebuilt, which might be an issue if you add a lot of blocks to a Neo field.

@benjamminf benjamminf self-assigned this May 19, 2016
@benjamminf benjamminf added this to the 1.2.0 milestone May 19, 2016
@benjamminf
Copy link
Contributor

It seems rebuilding the structures is not the bottleneck, nor does it impose any significant delay when saving.

I attempted caching as well, and while it does shave off a few hundred milliseconds in rendering, saving the field is still the main bottleneck.

I'm going to keep looking into this.

@benjamminf
Copy link
Contributor

With a comparative Matrix field and value, the Neo counterpart performs worse, taking about twice as long to save. Just ruling out it being something to do with Craft itself. That said, a Matrix field can still take a very long time to save as well. I have a feeling the time discrepancy may be explained by the fact that Neo uses the content table instead of creating it's own tables for each field.

@benjamminf
Copy link
Contributor

benjamminf commented Jun 10, 2016

Here's an interesting idea:

Neo fields could auto-save their changes while you're editing them via AJAX, but linked to a temporary element. Then when you save the real element, instead of having to perform a tonne of DB queries to save the Neo field, it just changes the content reference from the temporary element to the real one. This means that it'd effectively take no time at all to save a Neo field.

It'd also mean having to fire a task periodically to clean up any temporary elements (which would be left around if someone were to edit a Neo field and then discard the changes).

As of version 1.2.0 I've locked down the DB schema for Neo, so something like this would have to wait for version 2.x.

@benjamminf
Copy link
Contributor

Another idea is simply only perform DB queries on changed data. That should see a very noticeable increase in performance.

@benjamminf
Copy link
Contributor

benjamminf commented Jun 13, 2016

(Just some more testing results to my own future reference)

  • The actual process of saving the Neo field into the DB isn't the bottleneck.
  • Rendering the field is the bottleneck, but it's somehow tied to saving the field in some unknown way.
  • Caching the input field sees a very noticeable performance increase.
  • Saving a field after caching still performs as worse as it would without caching.
  • Disabling all saving functionality, while also caching, still performs badly
  • On top of previous point, then removing all post data, so it has no data to work with at all, still performs badly.

Something very strange is going on under the hood of Craft, or PHP for that matter. Experimentation seems to split the bottleneck between rendering and saving, but disabling one doesn't affect performance at all, but disabling both does.

@benjamminf
Copy link
Contributor

benjamminf commented Jun 13, 2016

So I finally decided to crank out the xDebug profiler, and low-and-behold the culprit is the NeoFieldType::getSearchKeywords method, taking up a whopping 84% of the total saving time. The remaining 16% goes mostly to saving each block. I've been experimenting with only saving blocks that have been modified, and so far it works very well.

In practice, disabling generating search keywords see's around a 45% decrease in saving time which, while very good, is not in line with the results from profiling. I'm fairly certain this is due to rendering the field again after saving, so caching that should really speed up the saving time (should cache on a per-block basis). I also need to figure out how to spin off generating search keywords into a separate task.

Looks like I finally have a plan in place. Should even out-perform the Matrix field.

@shornuk
Copy link
Author

shornuk commented Jun 15, 2016

Good news, and great work on this. Thanks!

@benjamminf
Copy link
Contributor

Cheers dude. I've got keywords being generated in a task fully implemented, and now even entries that have like 50 blocks filled with content save almost as quickly as an entry with nothing at all. Still got problems with the block caching, but once that's sorted I'll be releasing 1.2.2 with all these speed improvements.

@benjamminf benjamminf modified the milestones: 1.2.2, 1.3.0 Jun 16, 2016
@shornuk
Copy link
Author

shornuk commented Jun 16, 2016

Sounds awesome.!

benjamminf added a commit that referenced this issue Jun 16, 2016
Resources included with blocks are now filtered, and only get added to
the DOM if they don't already exist there. This dramatically improved
render performance for cases where there are a lot of resources included
with blocks and their fields. Could perhaps improve on this further by
asyncronously loading JS resources
@benjamminf
Copy link
Contributor

benjamminf commented Jun 19, 2016

Everything is finalised and has passed all testing, but I will be using it in the wild for at least the next week before I release. If you'd like to try out the new performance enhancements now, you can grab a copy of the plugin here.

@tyler-deboer
Copy link

I've been seeing this too - I really love the plugin and the flexibility it's giving me with my layouts, but it's been slowing my control panel to a halt. Granted, I have a few fairly large pages that I'm working on - smaller ones (predictably) don't affect CP speed/performance nearly as much. And it does look like the culprit is the getSearchKeywords tasks that run every on every save - it's running literally dozens of times every time I save something, and has gotten stuck a couple times.

This isn't a deal-breaker by any means for me (and I know you're working on fixing this!), but would I be losing any core functionality if I just stripped out the getSearchKeywords function in the NeoFieldType.php file? I guess I'm not sure what that is actually doing.

Thank you for all you work on this, so far it's been a huge asset for me and my team!

@benjamminf
Copy link
Contributor

benjamminf commented Jun 29, 2016

Hmm that is interesting. Since 1.3.0 it really shouldn't be slowing down the CP much at all, even with large fields. There is still a noticeable impact when rendering large fields – this is because there's simply megabytes of generated HTML and potential external resource files provided by third-party field types. Regardless of what I do I can't get around that bottleneck (actually this might have spawned an idea, I'll get back to you on this).

As for the search keyword tasks, I'm not sure how dozens of them are spawning after a single save, unless you have dozens of Neo fields on a single entry. One task is spawned for one Neo field. I haven't experienced stuck tasks either, but I suppose that's bound to happen given Craft's task system (happens a lot with the "Deleting stale caches" task). How large are these fields? Average number of blocks, block types and fields per block type?

Search keywords are used in the CP when searching for entries, categories, etc, as well as on the front-end when querying for entries using the .search() filter. If you don't require having any of the content in your Neo fields searchable, then it's safe to disable generating these keywords. You can disable the "Generating search keywords" task by opening neo/fieldtypes/NeoFieldType.php, finding the getSearchKeywords method (line 539) and removing the craft()->tasks->createTask() service call. That said, tasks are run asynchronously so I don't think this is what's actually slowing down the CP.

Unfortunately even with Matrix fields you're going to see these performance impacts. There's a lot of overhead when adding Craft and Yii into the mix. Craft 3 has seem some big performance boosts in the database querying department so hopefully this doesn't stay a problem for much longer. If you haven't already, I'd strongly recommend upgrading your server's PHP version to 7, as it also performs much better than any 5.x version.

@tyler-deboer
Copy link

So after some time testing and trying to isolate the slowdown issues, I'm pretty sure the culprit is one (or more) of my Chrome extensions. If I load and edit entries in the CP in Chrome Incognito mode or Firefox/Safari, the slowdowns are pretty much gone (it still takes a few seconds to initially load the page which is certainly acceptable, but I was getting load times up to a minute on my normal Chrome). I'm going to look into just which extensions are causing the problems. I was getting all hundreds of console errors in my normal Chrome when I would try to click around before giving the entry edit page time to load completely:

VM20314:14 GET data:image/gif;base64,R0lGODlhw30BAAD/ACwAAAAAw30BAAACAkQBADs= net::ERR_INSUFFICIENT_RESOURCES

I don't get those at all on Incognito or other browsers, which makes me suspect it's an extension that's not playing nicely.

And also, I don't think this is a big deal, but I do get some console warnings when I add in a block that contains a Super Table text field: 25garnish-0.1.js:225 Double-instantiating a transparent text input on an element. Again I don't think that is causing any problems but it's something that I thought I would mention at least.

I appreciate your help with this though! I will keep you posted if I can isolate just which extension is causing the problem (in case others are running into similar problems).

@benjamminf
Copy link
Contributor

Wow, minute load times are definitely out of the ordinary. It does sound like an extension is playing havoc with Neo, but I'm not sure why that could be. It seems that whatever is going on, a lot of resource requests were being fired, until the point that Chrome ran out of memory (hence the net::ERR_INSUFFICIENT_RESOURCES message). That would also explain the long load time and console errors.

As for the warning, it's a minor warning by Garnish that doesn't really affect anything. In certain situations some Garnish components are being initialised twice, but Garnish picks up on it and handles it. This happens usually because when Neo blocks are created, Neo initialises all Garnish components inside the block, but some fields will explicitly initialise their components as well. So sometimes you get these warnings, but everything still functions normally.

No problem, thanks for the heads up. If you figure out what extension is causing this problem, I'll see if I can figure out why and hopefully fix it 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants