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

Fix out of memory issue when having 50+ rules and 100k+ products #43

Merged
merged 2 commits into from
Nov 3, 2019

Conversation

joost-florijn-kega
Copy link
Contributor

Fixes #42

The reason for using a iterator instead of the fetchAll of the connection is that the iterator won't load the whole collection in memory at once.

@krehberg
Copy link

krehberg commented Jul 12, 2019

Thanks for the effort @joost-florijn-kega,

but when i use your code, i get an exception when i do "php bin/magento setup:di:compile"

[ReflectionException]
Class Faonni\SmartCategory\Model\Rule\Condition\ProductCategoryList does not exist

we are running magento 2.2.7

how can this be fixed, if it is working for you?

am i missing something?

@karliuka
Copy link
Owner

karliuka commented Jul 14, 2019

Thank You very much.
I think that if you add the line
use Magento\Catalog\Model\ProductCategoryList;
to the
Faonni/SmartCategory/Model/Rule/Condition/Product.php
file, the error will be fixed.

@krehberg
Copy link

So, i took the code and it works.
428 Rules later (and 160 to go), which took a day or so, it uses currently 1.516.503.040 Bytes of RAM for PHP.
This is much better than before, but of course, the speed suffered from the change.
And the "leak" is not completely gone. The RAM usage never drops below the previous value (measured with memory_get_usage(true)).

But hey, it works, which is more then just good enough.

When i find the time to dig deeper, maybe i will try to improve it. But thats for another day.

Thank you very much!

@krehberg
Copy link

After some weeks of usage, it turns out that everything works fine.
The speed is not great, but instead of getting the process killed, after it uses all the RAM, it now finishes successfully.
So, thanks for the fix again @joost-florijn-kega

Could you please merge it in the master and create a new release @karliuka

Thanks you!

@karliuka karliuka merged commit fc7a876 into karliuka:master Nov 3, 2019
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

Successfully merging this pull request may close these issues.

Memory Leak - massive memory usage
3 participants