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

Only disable elements that are not already disabled (can massively speed up completion) #1241

Closed
kristiansp opened this issue Feb 18, 2023 · 4 comments
Labels

Comments

@kristiansp
Copy link
Contributor

Description

When feed is set to disable elements, every single element is explicitly set to $element->enabled = false; and then saved, even if it already was disabled. When keeping disabled elements around, this can massively slow down the feed completion, as well as using unnecessary resources resaving a lot of elements.

A simple if to check if an element is already disabled, and then only save it if it is enabled, sped up my feed completion from 44.82s to 2.25s(!), although mileage of course may very. (I use Feed Me to sync content from an external source, running every hour, so most of the time there are no big changes).

Even if the gains are not so huge in every scenario, it seems a little bit like a simple no-brainer that can't really negatively impact anyone.

I already have a PR for this from August 2020:
#736

Steps to reproduce

  1. Set up feed with import strategy to disable missing elements
  2. Have a substantial amount of disabled elements
  3. Wait, wait and wait for the import to complete while it's disabling already disabled elements

Additional info

  • Craft version: 4.3.8.2
  • PHP version: 8.0.26
  • Database driver & version: MySQL 10.4.27
  • Plugins & versions: FeedMe 5.0.5 (but basically any version)
@kristiansp kristiansp added the bug label Feb 18, 2023
@kristiansp
Copy link
Contributor Author

kristiansp commented Feb 18, 2023

I see that the old pull request has a conflict now because of extra parameters in the saveElement method. But it should suffice to go from this in src/base/Element.php:

$element->enabled = false;
$elementsService->saveElement($element, true, true, Hash::get($this->feed, 'updateSearchIndexes'));

to this:

if ($element->enabled) {
    $element->enabled = false;
    $elementsService->saveElement($element, true, true, Hash::get($this->feed, 'updateSearchIndexes'));
}

@angrybrad
Copy link
Member

Resolved in #1248

@brandonkelly
Copy link
Member

Feed Me 4.6.0 (Craft 3) and 5.1.0 (Craft 4) have been released with this change (via #1248) 🎉

@kristiansp
Copy link
Contributor Author

Amazing, thanks!

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

No branches or pull requests

3 participants