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

Possible additional fix #4

Open
convenient opened this issue Aug 3, 2021 · 5 comments
Open

Possible additional fix #4

convenient opened this issue Aug 3, 2021 · 5 comments

Comments

@convenient
Copy link

Hello

When Magento is handling numbers of requests at the same time (e.g. some headless frontend sent 10 requests at the same time on page load to ask graphql api for data) then each request got information to start building a cache. If cache exists - everything works perfectly but if it doesn't then first request starts building the cache and each next request do the same too.

I saw this exact same issue with a PWA setup and the graphql endpoint, the more custom schema we added the worse performing it got which caused this issue.

In this PR I improved the schema stitching algorithm to fix the performance issue, we have it on several production sites without issue (known issue is that magento schema stitching functions have a buggy regex with union schema types, but that doesnt affect us). magento/magento2#31879

Just thought you might be interested in this.

@maritos
Copy link
Owner

maritos commented Aug 5, 2021

Wow, I heard about this solution from my good friend. Thanks for sharing, I gonna check it when I get the chance 🙂

@ByJacob
Copy link

ByJacob commented Aug 24, 2021

I am waiting for an update on this patch for this package :)

@maritos
Copy link
Owner

maritos commented Jan 12, 2022

@convenient ech I just noticed that your PR is not merged yet! I would say that it is not even close given the adobe core time activity there. Shame on Adobe, seems like Adobe don't care.. on top of you did good job.

Anyway I think my change is not the same type of solution and you can still use this repo to cover some potential issues with concurrency however with your fix the performance is not that much painfull.

Your changes only shows how much the system could be optimized if we only want to OR how ineffective it is written in some places.

TBH it is more PHP issue in terms of handling concurency and even 0.2s or 1s script has the problem. However the performance demage is more accceptable for real life cases.

Since PHP 8.1 there is new interesting thing added to PHP called Fibers > https://wiki.php.net/rfc/fibers
which could be a good response for that type of issues, but I must investigate this :)

Plus I am going to check solution pointed here #5 by @onlinebizsoft

Thanks
Mariusz

@convenient
Copy link
Author

@maritos just a FYI that my pull request has just been merged :)

Agreed our solutions are different, could be complementary maybe

@maritos
Copy link
Owner

maritos commented Jan 14, 2022

@convenient wow it is big news. Finally they merged it! 🚀 I noticed it is a fresh case from 4 hours ago. 🎉

Gratz and thanks for your big effort!

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

No branches or pull requests

3 participants