Performance can be increased significantly by firing multiple concurrent requests. With my provided patch I could decrease execution time locally by ~2/3.

CommentFileSizeAuthor
#15 3152000-15.patch7.02 KButor.io
#13 3152000-13.patch7.46 KButor.io
#12 3152000-12.patch8.13 KButor.io
#9 3152000-9.patch6.69 KButor.io
#7 3152000-7.patch6.7 KButor.io
warmer_async_requests.patch6.34 KButor.io
#4 3152000-4.patch7.63 KButor.io

Comments

arthur_lorenz created an issue. See original summary.

Status: Needs review » Needs work

The last submitted patch, warmer_async_requests.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

utor.io’s picture

utor.io commented
utor.io’s picture

utor.io commented
StatusFileSize
new 3152000-4.patch7.63 KB

Fixed some CS issues, added default config and imroved naming. Unfortunately I can't get the tests to run properly on my local environment right now, so let's hope tests are green :D

utor.io’s picture

utor.io commented
Status: Needs work » Needs review
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
Status: Needs review » Needs work

This is awesome. I have been meaning to address this for a while. I wonder if we can remove the TODO item in favor of creating a ticket, or fixing it here. I have second thoughts about making the assync feature configurable, maybe we should make it always async if that is available to all Drupal installs.

Back to needs work to get clarification about the questions above.

utor.io’s picture

utor.io commented
Status: Needs work » Needs review
StatusFileSize
new 3152000-7.patch6.7 KB

Alright, I made the async requests mandatory, removed the TODO (feel free to create a ticket for it :)) and added a message after successful cache warming.

Status: Needs review » Needs work

The last submitted patch, 7: 3152000-7.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

utor.io’s picture

utor.io commented
Status: Needs work » Needs review
StatusFileSize
new 3152000-9.patch6.69 KB

Oh, I used a function that's only available for php >= 7.3. Fixed that

Status: Needs review » Needs work

The last submitted patch, 9: 3152000-9.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented

Any guesses on why tests are failing?

utor.io’s picture

utor.io commented
Status: Needs work » Needs review
StatusFileSize
new 3152000-12.patch8.13 KB

Yes, that was caused by a missing onReject callback. Fixed this and also fixed some CS issues.

utor.io’s picture

utor.io commented
StatusFileSize
new 3152000-13.patch7.46 KB

Whoopsie, deleted composer.json...

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented

I really like this change. My only question is weather or not we want to drop the Make asyc requests checkbox, and only allow configuring the number of parallel requests.

I will let my co-maintainer decide on that sense, unless there is a technical point I am missing that you Arthur want to clarify in that regard.

Thanks again for this awesome patch!!

utor.io’s picture

utor.io commented
StatusFileSize
new 3152000-15.patch7.02 KB

No there is no technical reason for not making async requests mandatory. I just wasn't sure how positive your reception to this change will be, so I decided to go with a smooth introduction of this feature :D

Updated the patch to get rid of that option completely.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented

This looks good. There is a debug log statement left by accident that I may remove on commit.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
Status: Needs review » Fixed

This was merged to the new release with minor changes.

Status: Fixed » Closed (fixed)

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