Saving a view causes the entire cache to be invalidated

I can confirm that this problem still exists. Saving a view requires up to 16 GB (!!) of memory on my page. This is not really nice ;)

In our instance, variable_set('menu_rebuild_needed', TRUE); that's called in views_invalidate_cache() is the killer and isn't addressed in the patch. Looking for a solution other than commenting that out as discussed in #2263557: Slow performance when saving views which I closed as a dupe of this.

The complete menu rebuild is also an issue. But it is the only way to get changes in views displays that have a path (such as pages) to take inmediate effect.

Maybe a quick solution would be to only invalidate the menu cache if:

- Any of the existing views is of type "page" (or any display that uses a path) and the path has changed.
- A display of the current view has been added/removed, and it uses paths.

This last solutions does not look that bad, as it will only invalidate the menu cache if anything in the view has changed that afects it.

Is there anything I can do to help with this issue? I have a few sites where the saving of a view takes about 200 years. I didn't realize that each save forces such a clearing of any cache systems.

@Shane Birley can you confirm this patch saves a few years?

This patch looks like it would be very useful. Is there anything missing where it would miss things that need to be cleared?

A rough performance test with just devel, saving a view that didn't change changed (opened the title and hit ok to get the save button).

Xdebug was turned off:

Without patch
  - Executed 3215 queries in 1516.48 ms. 
    Page execution time was 10013.1 ms. 
    Memory used at: devel_boot()=2.39 MB, devel_shutdown()=118.12 MB, PHP peak=148.75 MB.
  - Executed 3211 queries in 1548.17 ms.
    Page execution time was 10034.37 ms. 
    Memory used at: devel_boot()=2.39 MB, devel_shutdown()=118.17 MB, PHP peak=148.25 MB.

With patch
  - Executed 3314 queries in 1610.93 ms. 
    Page execution time was 9338.66 ms. 
    Memory used at: devel_boot()=2.39 MB, devel_shutdown()=122.71 MB, PHP peak=153.75 MB.
  - Executed 3318 queries in 1605.3 ms. 
    Page execution time was 9080.62 ms. 
    Memory used at: devel_boot()=2.39 MB, devel_shutdown()=122.61 MB, PHP peak=153.5 MB.

And to the point about variable_set('menu_rebuild_needed', TRUE); Just disabling that with the patch and no xdebug:

  - Executed 65 queries in 27.72 ms. 
     Page execution time was 528.3 ms. 
     Memory used at: devel_boot()=2.32 MB, devel_shutdown()=46.79 MB, PHP peak=47.25 MB.
  - Executed 69 queries in 32.64 ms. 
     Page execution time was 547.86 ms. 
     Memory used at: devel_boot()=2.32 MB, devel_shutdown()=46.88 MB, PHP peak=47.5 MB.

Summary

This patch saves me roughly ~700ms-1s. Avoiding the menu rebuild saved ~8.5 seconds from that.

So the bigger win would go to avoiding the menu rebuild. Are there any conditions we could check like (path changed on view, or other things that would require a menu rebuild)?

Menu path doesn't change often so this could be huge.

or work on core menu rebuilding not taking so long:)

@joelpittet: Regarding the rebuilding of menu items, give #1978176: Build menu_tree without loading so many objects a glance. There was a patch there that helped me a LOT but it still needs some work.

@torgosPizza I'm on that issue, a bit worried about #1978176-68: Build menu_tree without loading so many objects #68 David R's comment about security.

Thanks for pointing at it again though. I should really take a stab to see my savings!

Using the patch @torgosPizza mentioned in #9 with xdebug off and doing the same quick devel page test of saving a view on no changes and with the patch applied.

Run #1
Executed 3689 queries in 1830.8 ms. 
Page execution time was 11346.3 ms. 
Memory used at: devel_boot()=2.53 MB, devel_shutdown()=131.54 MB, PHP peak=169.5 MB.
Run #2
Executed 3314 queries in 1573.32 ms. 
Page execution time was 9163.66 ms. Memory used at: devel_boot()=2.43 MB, devel_shutdown()=123.01 MB, PHP peak=159.25 MB.

The page execution time was roughly the same. So unfortunately no gain was made with that other issue's patch for me.

Yeah, the earlier one was the one that actually got me decent results (comment #32). I guess it's one of those YMMV situations!

I understand that it is only the menu_rebuild that yields drastic results but as for the original patch and issue itself I think it does make sense to only invalidate the cache for a single view. Possible to RTBC this patch and leave the rest to #1978176: Build menu_tree without loading so many objects?

I'm going to RTBC this because I've been using it for quite some time and it was helping me out:)

Actually, with the latest dev it needs a re-roll, then RTBC:)

Re-roll of #1

Mhm any idea why the tests fail? I can confirm it was RTBC and working before...

Re-triggering

The tests are expecting more is cleared than are, or we need to clear some other cid as well.

This patch helped us, as we've been dealing with downtime every time I save one particular View. For me it's RTBC but to @joelpittet's comment I'd like to see what we can do to get the tests to turn green.

+1 for #24. In case the view display has a menu entry perhaps the menu cache also has to be cleared? Might that cause the tests to fail?

I've just been running into a new downtime again because this patch was not commited... bad trouble with the customer. For large sites this is a real problem causing downtimes so I'm now pushing the priority. I also retriggered to test and will have a look at the reasons why it still fails afterwards.

we're also using this patch #17 because it's such a huge improvement on save speeds, (despite the test failures) , not sure why the test failures.

This will solve the test failures but I'm not sure what caches need to be cleared and which ones are fine to leave on save and delete?

Thank you joelpittet, the patch looks good so far. But it should also consider to only set

variable_set('menu_rebuild_needed', TRUE);

if a menu related setting has been changes or the view display type is menu related (like "page"). Otherwise the menu rebuild is extremely painful and expensive. It also locks the database until the rebuild is done.

@Anybody, that sounds like a nice addition but maybe a scope creep on this issue. There is no context provided to that function to help tell if the view has a menu or not.

The bigger question to get this in is "How do we know we've cleared enough from cache_views"? There could be a contrib that potentially relies on that clear to function which could be a blocker for this.

@joelpittet: Thank you for your quick reply. You are right that one main question is "How do we know we've cleared enough from cache_views"? but I'm sadly not able to answer that question. Should we perhaps contact a views maintainer for that?

But furthermore I think that the menu rebuild point is not a nice addition but a very important point. On large websites that is a very huge problem and as far as we can tell even the largest problem of all at this point. Currently we even have to comment out that line on pages with > 1 mio nodes. And that page uses panels etc. to include views instead of views pages. I think that menu related views (=views page display type) are at least < 50% of all views display types used.
But of course we may not introduce wrong or missing functionality here but I think that function call has to be much more granular perhaps at a very different point int the views display definition, perhaps using a hook?
If you ask me, that call should have never been placed at that point.

So how can we go on and who can we ask best?

If you agree I'd like to place these two last points on the agenda to finally fix this old issue.

@Anybody, the menu_rebuild_needed point could be it's own issue.

Not sure who to ask.

RTBC+1 patch 17

Based on #14, but with patch #29
Keep scope small, see Sylus comment #14
Huge performance improvement

3.19 was just released, somehow we missed including this :(
patch has a significant performance implication as mentioned previously
Check to make sure if the patch needs a reroll or not.

Sorry for missing this.

Lets get it into the next stable release.

Committed. Thanks.