Remove the eager and lazy loading options
#333
Comments
|
I think we should use this time to elaborate using doctrine ORM instead and forget about our own model implementation. |
|
No, this is an entirely different topic. We need a fix for the current implementation (see contao/core#7928). |
|
Doctrine ORM uses a lot of memory as well. We always refrained from using it in Symfony projects where a lot of data was handled. |
|
Every ORM uses a lot of RAM if not handled correctly... |
|
You have used it a lot without knowing, because it has been defined in the DCA :) |
|
I mean in my own implementations. Obviously I know where it's defined and how it works. |
|
Well the most simple solution would be to set all core DCA's to |
Not sure how you could handle such a simple case incorrectly. Sure, any ORM will cause a much higher memory footprint, but the advantage of Contao's Model is, that its footprint is comparatively lightweight - unless a related model has to be loaded. Then its memory consumption explodes (where as with Doctrine ORM, it would stay lower in comparison). |
That does not really make sense to me. Why would that be the case? |
|
I did a little analysis some time ago: https://community.contao.org/de/showthread.php?52268-Newsarchiv-mit-%FCber-10-000-Eintr%E4gen-sprengt-Speicher&p=337337&viewfull=1#post337337 |
|
That certainly should not be the case since we have a model registry. If that's really true then its a bug we must fix. |
It does not work indeed. It's easy to test. Just put an echo here: Model.php#L163, e.g. \NewsModel::findPublishedByPids(…);You will get Skimming over the code, I think this is because the news archive is never actually written into the registry at any point when loading news entries via \NewsArchiveModel::findById(…);right before loading the news entries via the If this is fixed, then the proposed change in this issue might not even be necessary (but could still be useful of course). Without the bugged generation of the related model, the (peak) memory consumption of And this is also something that can be fixed in Contao 3.5 without breaking the backwards compatibility. I can open a separate ticket for that if necessary. |
|
We have just discovered a bug in the |
|
👍 looks good. With the faeea6e fix the memory consumption drops from 187.2 MiB to 66.9 MiB :) (with my 10,000 news entries dataset) (actually it dropped to 66.9 MiB from 245.5 MiB just now in my testing environment... the peak memory consumption is weirdly inconsistent when filling up the memory with 10,000 of these |
|
So this now means each item is using about 6.85056kb RAM. Still a bit much but at least somewhat reasonable. |
|
Back-ported in contao/core@8ede060. |
Version 3.5.3 (2015-09-10) -------------------------- ### Fixed Correctly handle dimensionless SVG images (see #7882). ### Fixed Correctly fill in the image meta data in news, events and FAQs (see #7907). ### Fixed Enable the `strictMath` option of the LESS parser (see #7985). ### Fixed Consider the pagination menu when inserting at the top (see #7895). ### Fixed Use en-dashes in event intervals (see #7978). ### Fixed Store the correct edit URL in the back end personal data module (see #7987). ### Fixed Adjust the breadcrumb trail when creating new folders (see #7980). ### Fixed Use `$this->hasText` in news and event templates (see #7993). ### Fixed Convert the HTML content to XHTML when generating Atom feeds (see #7996). ### Fixed Correctly link the items in the files breadcrumb menu (see #7965). ### Fixed Handle explicit collations matching the default collation (see #7979). ### Fixed Fix the duplicate content check in the front end controller (see #7661). ### Fixed Correctly parse dates in MooTools (see #7983). ### Fixed Register the related models in the registry (see contao/core-bundle#333). ### Fixed Correctly escape in the `findMultipleFilesByFolder()` method (see #7966). ### Fixed Override the tabindex handling of the accordion to ensure that the togglers are always focusable via keyboard (see #7963). ### Fixed Correctly generate the news and event menu URLs (see #7953). ### Fixed Check the script when storing the front end referer (see #7908). ### Fixed Fix the back end pagination menu (see #7956). ### Fixed Handle option callbacks in the back end help (see #7951). ### Fixed Fixed the external links in the text field help wizard (see #7954) and the keyboard shortcuts link on the back end start page (see #7935). ### Fixed Fixed the CSS group field explanations (see #7949). ### Fixed Use ./ instead of an empty href (see #7967). ### Fixed Correctly detect Microsoft Edge (see #7970). ### Fixed Respect the "order" parameter in the `findMultipleByIds()` method (see #7940). ### Fixed Always trigger the "parseDate" hook (see #4260). ### Fixed Allow to instantiate the `InsertTags` class (see #7946). ### Fixed Do not parse the image `src` attribute to determine the state of an element, because the image path might have been replaced with a `data:` string (e.g. by the Apache module "mod_pagespeed").
As e.g. stated in contao/core#7928, the model handling needs to be fixed and optimized.
Eager and lazy loading
Right now it is possible to pass
['eager' => true]as an option to enforce eager loading of related models. However, it is not possible to pass['eager' => false]to enforce lazy loading.Retrieving related models
Currently the query builder generates a JOIN statement and adds the related columns to every row. The advantage of this approach is that there is only one database query. The disadvantage is that the amount of data sent is quite big.
A possible solution
To solve both issues, we should simply remove the
eagerandlazyoptions from the DCA and always use lazy loading unless['eager' => true]is explicitly set. It will keep the queries small and it will load existing models from the registry instead of creating them from the DB result.@contao/developers What do you think?
The text was updated successfully, but these errors were encountered: