(@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.
Comment | File | Size | Author |
---|---|---|---|
#1 | menu-link-resave-2100753-1.patch | 572 bytes | Berdir |
Comments
Comment #1
BerdirComment #2
BerdirTestbot runtime. 1h18. Most tests on that bot seem to be around 1h45. Doing a re-test to get some more results.
Comment #3
Berdir#1: menu-link-resave-2100753-1.patch queued for re-testing.
Comment #4
amateescu CreditAttribution: amateescu commentedI 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 :(
Comment #5
catchI 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.