Problem/Motivation

I recently worked on a project that heavily relied on the feeds module, I realized when feeds import was happening it took more than normal to get a response on any page, and I thought maybe making the http fetch request asynchronous would somehow help improve this.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork feeds-3219744

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Bahson created an issue. See original summary.

bahson’s picture

bahson commented
Assigned: Unassigned » bahson
megachriz’s picture

megachriz
Primary language Dutch
commented

Can you explain how that would work? Feeds cannot import content if it hasn't been fetched yet. And you can already configure Feeds to run imports during cron runs, so I assume the webserver would already fetch in a separate request in this case.

bahson’s picture

bahson commented

@megachriz, here is a thought that inspired this;
Let's assume the feeds source (website) response time is 20 secs (maybe lower), and you have like 10+ sources to import from or at least check if there's something new.
Making this asynchronous would mean, maybe less execution time than waiting (20 sec) * (number of sources).
Not sure if other parts need to be tweaked to achieve this, but I hope to test first even the initial idea.

Bahson opened merge request !25

bahson’s picture

bahson commented
Status: Active » Needs review
bahson’s picture

bahson commented

@MegaChriz, sure the content imports correctly,
About performance improvement, let me re-run the imports severally with current and previous implementation and report back (will edit this comment with the answer to the last question.)
Update:
About performance improvement, it sure does improve: I can get a response faster on the homepage (views listings with freshly imported content), than with previous implements specifically when cron is running.

megachriz’s picture

megachriz
Primary language Dutch
commented

Okay, good :)

I'm planning to try it. I hope I can get to that soon. Feel free to post a new comment if I haven't done so in a few weeks from now.

bahson’s picture

bahson commented

Awesome, I sure will sometime after August.

megachriz’s picture

megachriz
Primary language Dutch
commented

So I tested this change, but I didn't notice any improvements on the amount of time it took to import.

This is what I did:

  1. I created four feed entities for importing RSS feeds. Each feed had 37 - 50 items each. All feeds where of the same feed type.
  2. On the HTTP source side, I delayed returning the content with 20 seconds by using sleep(20);.
  3. I scheduled imports for all four feeds.
  4. On the command line I run cron twice in a row: drush core-cron;drush core-cron.

Without the code change, the import of all four feeds took about 1 minute and 36 seconds. I rerun the test a few times to make sure that this import time was constant.
And with the code change, the import still took 1 minute and 36 seconds.

So I assume that the performance improvement is elsewhere? I see you wrote that the homepage response was faster. So does that mean that a cron run/feeds import makes the site a bit slower?

I'm not too experienced with Guzzle, but looking at the docs about async requests, would the import times perhaps speed up more if using a callback on the then() method?

megachriz’s picture

megachriz
Primary language Dutch
commented

I could of course also merge it as is. While I noticed not any direct improvements, nothing did break either.

bahson’s picture

bahson commented

Yeah, the import should take the same time, but the improvement is somehow subtle, in that, the request part is not blocking, and other scripts can be executed as the cron fetch request is being processed.

  • MegaChriz committed 67c3563 on 8.x-3.x authored by Bahson 
    Issue #3219744 by Bahson, MegaChriz: Make http fetch requests...
megachriz’s picture

megachriz
Primary language Dutch
commented
Status: Needs review » Fixed

Although the speeds of imports could probably be improved more, I decided to just merge this now. I don't think I'll get to focus on improving the import performance anytime soon and this change would then have to wait a long long time.

Thanks for contributing!

bahson’s picture

bahson commented

You're most welcome, and thanks to you too, for the feeds module to be precise.

Status: Fixed » Closed (fixed)

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