(@amateescu, I chose menu_link.module as component!)

menu_link.module has this snippet to only save menu links that changed.

// If every value in $entity->original is the same in the $entity, there
      // is no reason to run the update queries or clear the caches. We use
      // array_intersect_key() with the $entity as the first parameter because
      // $entity may have additional keys left over from building a router entry.
      // The intersect removes the extra keys, allowing a meaningful comparison.
      if ($entity->isNew() || (array_intersect_key(get_object_vars($entity), get_object_vars($entity->original)) != get_object_vars($entity->original))) {
       // save happens here.
     }

Unfortunately, it doesn't work.

If you put the following inside the if:

debug(DiffArray::diffAssocRecursive(array_intersect_key(get_object_vars($entity), get_object_vars($entity->original)), get_object_vars($entity->original)), 'DIFF');

You get the following output:

array (
  'plid' => 0,
  'hidden' => 0,
  'external' => 0,
  'expanded' => 0,
  'weight' => 9,
  'depth' => 1,
  'customized' => 0,
  'p2' => 0,
  'p3' => 0,
  'p4' => 0,
  'p5' => 0,
  'p6' => 0,
  'p7' => 0,
  'p8' => 0,
  'p9' => 0,
  'route_parameters' => NULL,
)

route_parameters in the original is not unserialized. So it's NULL vs. 'N;' The others are string vs. integer, where our diffAssocRecursive does type safe checks but != does not, so that's ok. (You can also use array_diff_assoc() but that prints a million notices on PHP 5.4+)

Got quite confused for a moment because i couldn't reproduce it with a manual load/save snippet. Then I noticed that _menu_navigation_links_rebuild() does it's own custom ->original logic and when route_parameters was added, the additional unserialize wasn't added there.

The performance difference is impressive. "Clear all caches" is 40% faster, ~3000 queries less executed and 5MB less peak memory. Results may vary on other systems.

Patch coming in a second, not sure how to test this in an automated way.

CommentFileSizeAuthor
#1 menu-link-resave-2100753-1.patch572 bytesBerdir
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Status: Active » Needs review
FileSize
572 bytes
Berdir’s picture

Testbot runtime. 1h18. Most tests on that bot seem to be around 1h45. Doing a re-test to get some more results.

Berdir’s picture

#1: menu-link-resave-2100753-1.patch queued for re-testing.

amateescu’s picture

Status: Needs review » Reviewed & tested by the community

I remember reviewing the patch that added the route_parameters field and asking where does it unserialize it, so this line should've been there in the first place.. not idea how it got lost :(

catch’s picture

Status: Reviewed & tested by the community » Fixed

I worked on the original logic to save the extra saves. Shame we broke it but great that it's caught and fixed!

I don't see a way to test this unless we had automated performance testing with query logging as one of the measurements.

Automatically closed -- issue fixed for 2 weeks with no activity.