-
-
Notifications
You must be signed in to change notification settings - Fork 315
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
Eager load associated records #1674
Conversation
708b168
to
703a23c
Compare
If the contents have been eager loaded so we should not use SQL to find them. Even if they have not been eager loaded, they will be loaded on demand. Anyway it is better to not always load them via SQL.
This removes an unnecessary query while rendering elements.
In order to benefit from eager loading we need to set the inverse ofs on the page element associations. Usually active record is able to figure them out, but because we use a class_name we need to set this reverse-look-up ourselves.
The former implementation loads a new page from the database, instead of using the already loaded page from the controller. Only load a page if we pass a page layout string. This way we can take advantage of eager loading and reduce the amount of objects in memory
0008da4
to
a87adf2
Compare
An ingredient association is a way to be able to eager load associated records of essence classes. The EssencePicture for example `belongs_to :picture`. In able to eager load we create an `belongs_to :ingredient_association` for you and add an ActiveRecord hack that skips eager loading on records not having such relation. That way we can load a page and include all elements, their contents, essences and their associated records as well. This should speed up admin pages with large element lists and API calls to pages with many elements a lot.
41fdfcf
to
c45a6dd
Compare
Code Climate has analyzed commit c45a6dd and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 98.3% (80% is the threshold). This pull request will bring the total coverage in the repository to 92.2% (0.1% change). View more on Code Climate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Le great speedup!
- Use contents settings for size in EssencePicture#picture_url [#1703](#1703) ([tvdeyen](https://github.com/tvdeyen)) - Remove title tag from preview elements [#1701](#1701) ([tvdeyen](https://github.com/tvdeyen)) - Remove custom JS logging [#1700](#1700) ([tvdeyen](https://github.com/tvdeyen)) - Remove demo locale files [#1699](#1699) ([tvdeyen](https://github.com/tvdeyen)) - Use alchemyPageSelect for Node page select [#1698](#1698) ([tvdeyen](https://github.com/tvdeyen)) - Cache menu partials [#1697](#1697) ([tvdeyen](https://github.com/tvdeyen)) - Update page tree to menu nodes Rake task [#1696](#1696) ([tvdeyen](https://github.com/tvdeyen)) - Validate nodes name if page is absent [#1695](#1695) ([tvdeyen](https://github.com/tvdeyen)) - Update the application layout installer template [#1691](#1691) ([tvdeyen](https://github.com/tvdeyen)) - Update note about missing user class [#1690](#1690) ([tvdeyen](https://github.com/tvdeyen)) - Use a Sprockets 3/4 manifest file [#1689](#1689) ([tvdeyen](https://github.com/tvdeyen)) - Use select2 for internal page link in link overlay [#1685](#1685) ([tvdeyen](https://github.com/tvdeyen)) - Do not consider nested elements "orphaned" [#1684](#1684) ([mamhoff](https://github.com/mamhoff)) - Destroy page-dependent elements [#1683](#1683) ([mamhoff](https://github.com/mamhoff)) - Add anchor link tab to link overlay [#1682](#1682) ([tvdeyen](https://github.com/tvdeyen)) - Ensure the apt/cache folder exists while installing [#1678](#1678) ([tvdeyen](https://github.com/tvdeyen)) - Cache apt packages between CI runs [#1677](#1677) ([tvdeyen](https://github.com/tvdeyen)) - Use select2 with AJAX search for essence page select [#1675](#1675) ([tvdeyen](https://github.com/tvdeyen)) - Eager load associated records [#1674](#1674) ([tvdeyen](https://github.com/tvdeyen)) - Add support for testing with multiple Rails versions [#1673](#1673) ([tvdeyen](https://github.com/tvdeyen)) - Page api pagination [#1672](#1672) ([tvdeyen](https://github.com/tvdeyen)) - Adjust select2 loading-more indicator [#1671](#1671) ([tvdeyen](https://github.com/tvdeyen)) - Test support fixes [#1669](#1669) ([tvdeyen](https://github.com/tvdeyen)) - Build fixes [#1668](#1668) ([tvdeyen](https://github.com/tvdeyen)) - Add Menus [#1667](#1667) ([tvdeyen](https://github.com/tvdeyen)) - Add a label component [#1666](#1666) ([tvdeyen](https://github.com/tvdeyen)) - Run bundle install on CI even if cache hits [#1665](#1665) ([tvdeyen](https://github.com/tvdeyen)) - Moves switch_language method into languages_controller. [#1664](#1664) ([tvdeyen](https://github.com/tvdeyen)) - Cache gems between CI runs [#1663](#1663) ([tvdeyen](https://github.com/tvdeyen)) - Remove production gems from local Gemfile [#1662](#1662) ([tvdeyen](https://github.com/tvdeyen)) - Touch contents updated_at column in pure SQL [#1661](#1661) ([tvdeyen](https://github.com/tvdeyen)) - Convert page editing user methods into AR relations [#1658](#1658) ([tvdeyen](https://github.com/tvdeyen)) - Ensure the admin locale is only set by available locales [#1655](#1655) ([tvdeyen](https://github.com/tvdeyen)) - Add a GitHub actions ci.yml [#1654](#1654) ([tvdeyen](https://github.com/tvdeyen)) - Adjust install generator to latest changes [#1649](#1649) ([tvdeyen](https://github.com/tvdeyen)) - Deprecate _view suffix of element views [#1648](#1648) ([tvdeyen](https://github.com/tvdeyen)) - Add a configurable logout method (default: delete) [#1647](#1647) ([delphaber](https://github.com/delphaber)) - Deprecate render_essence helpers [#1644](#1644) ([tvdeyen](https://github.com/tvdeyen)) - Deprecate element editors [#1643](#1643) ([tvdeyen](https://github.com/tvdeyen)) - Deprecate local options in essence editors [#1642](#1642) ([tvdeyen](https://github.com/tvdeyen)) - Ensure the EssencePage id regexp matches only numbers [#1641](#1641) ([tvdeyen](https://github.com/tvdeyen)) - Use EssencePage in contact forms [#1640](#1640) ([tvdeyen](https://github.com/tvdeyen)) - Add Alchemy::EssencePage [#1639](#1639) ([tvdeyen](https://github.com/tvdeyen)) - FEAT: Render message and warnings in element editor [#1637](#1637) ([tvdeyen](https://github.com/tvdeyen)) - Tackle Rails 6 deprecations [#1636](#1636) ([tvdeyen](https://github.com/tvdeyen)) - Preload assets in tests [#1635](#1635) ([tvdeyen](https://github.com/tvdeyen)) - Allow acts-as-list 1.0 [#1634](#1634) ([tvdeyen](https://github.com/tvdeyen)) - Add Sprockets manifest file to dummy app [#1632](#1632) ([tvdeyen](https://github.com/tvdeyen)) - Master now tracks 4.4.0.alpha [#1627](#1627) ([tvdeyen](https://github.com/tvdeyen)) - Fix Cell Migration to maintain positions [#1625](#1625) ([mamhoff](https://github.com/mamhoff)) - Cell Upgrader: Match quotation marks in cell name string [#1624](#1624) ([mamhoff](https://github.com/mamhoff)) - Cell Migrator: Maintain element order in fixed elements [#1623](#1623) ([mamhoff](https://github.com/mamhoff)) - Enhance cells upgrader to deal with render_elements from_page: x [#1622](#1622) ([mamhoff](https://github.com/mamhoff))
- Use contents settings for size in EssencePicture#picture_url [#1703](#1703) ([tvdeyen](https://github.com/tvdeyen)) - Remove title tag from preview elements [#1701](#1701) ([tvdeyen](https://github.com/tvdeyen)) - Remove custom JS logging [#1700](#1700) ([tvdeyen](https://github.com/tvdeyen)) - Remove demo locale files [#1699](#1699) ([tvdeyen](https://github.com/tvdeyen)) - Use alchemyPageSelect for Node page select [#1698](#1698) ([tvdeyen](https://github.com/tvdeyen)) - Cache menu partials [#1697](#1697) ([tvdeyen](https://github.com/tvdeyen)) - Update page tree to menu nodes Rake task [#1696](#1696) ([tvdeyen](https://github.com/tvdeyen)) - Validate nodes name if page is absent [#1695](#1695) ([tvdeyen](https://github.com/tvdeyen)) - Update the application layout installer template [#1691](#1691) ([tvdeyen](https://github.com/tvdeyen)) - Update note about missing user class [#1690](#1690) ([tvdeyen](https://github.com/tvdeyen)) - Use a Sprockets 3/4 manifest file [#1689](#1689) ([tvdeyen](https://github.com/tvdeyen)) - Use select2 for internal page link in link overlay [#1685](#1685) ([tvdeyen](https://github.com/tvdeyen)) - Do not consider nested elements "orphaned" [#1684](#1684) ([mamhoff](https://github.com/mamhoff)) - Destroy page-dependent elements [#1683](#1683) ([mamhoff](https://github.com/mamhoff)) - Add anchor link tab to link overlay [#1682](#1682) ([tvdeyen](https://github.com/tvdeyen)) - Ensure the apt/cache folder exists while installing [#1678](#1678) ([tvdeyen](https://github.com/tvdeyen)) - Cache apt packages between CI runs [#1677](#1677) ([tvdeyen](https://github.com/tvdeyen)) - Use select2 with AJAX search for essence page select [#1675](#1675) ([tvdeyen](https://github.com/tvdeyen)) - Eager load associated records [#1674](#1674) ([tvdeyen](https://github.com/tvdeyen)) - Add support for testing with multiple Rails versions [#1673](#1673) ([tvdeyen](https://github.com/tvdeyen)) - Page api pagination [#1672](#1672) ([tvdeyen](https://github.com/tvdeyen)) - Adjust select2 loading-more indicator [#1671](#1671) ([tvdeyen](https://github.com/tvdeyen)) - Test support fixes [#1669](#1669) ([tvdeyen](https://github.com/tvdeyen)) - Build fixes [#1668](#1668) ([tvdeyen](https://github.com/tvdeyen)) - Add Menus [#1667](#1667) ([tvdeyen](https://github.com/tvdeyen)) - Add a label component [#1666](#1666) ([tvdeyen](https://github.com/tvdeyen)) - Run bundle install on CI even if cache hits [#1665](#1665) ([tvdeyen](https://github.com/tvdeyen)) - Moves switch_language method into languages_controller. [#1664](#1664) ([tvdeyen](https://github.com/tvdeyen)) - Cache gems between CI runs [#1663](#1663) ([tvdeyen](https://github.com/tvdeyen)) - Remove production gems from local Gemfile [#1662](#1662) ([tvdeyen](https://github.com/tvdeyen)) - Touch contents updated_at column in pure SQL [#1661](#1661) ([tvdeyen](https://github.com/tvdeyen)) - Convert page editing user methods into AR relations [#1658](#1658) ([tvdeyen](https://github.com/tvdeyen)) - Ensure the admin locale is only set by available locales [#1655](#1655) ([tvdeyen](https://github.com/tvdeyen)) - Add a GitHub actions ci.yml [#1654](#1654) ([tvdeyen](https://github.com/tvdeyen)) - Adjust install generator to latest changes [#1649](#1649) ([tvdeyen](https://github.com/tvdeyen)) - Deprecate _view suffix of element views [#1648](#1648) ([tvdeyen](https://github.com/tvdeyen)) - Add a configurable logout method (default: delete) [#1647](#1647) ([delphaber](https://github.com/delphaber)) - Deprecate render_essence helpers [#1644](#1644) ([tvdeyen](https://github.com/tvdeyen)) - Deprecate element editors [#1643](#1643) ([tvdeyen](https://github.com/tvdeyen)) - Deprecate local options in essence editors [#1642](#1642) ([tvdeyen](https://github.com/tvdeyen)) - Ensure the EssencePage id regexp matches only numbers [#1641](#1641) ([tvdeyen](https://github.com/tvdeyen)) - Use EssencePage in contact forms [#1640](#1640) ([tvdeyen](https://github.com/tvdeyen)) - Add Alchemy::EssencePage [#1639](#1639) ([tvdeyen](https://github.com/tvdeyen)) - FEAT: Render message and warnings in element editor [#1637](#1637) ([tvdeyen](https://github.com/tvdeyen)) - Tackle Rails 6 deprecations [#1636](#1636) ([tvdeyen](https://github.com/tvdeyen)) - Preload assets in tests [#1635](#1635) ([tvdeyen](https://github.com/tvdeyen)) - Allow acts-as-list 1.0 [#1634](#1634) ([tvdeyen](https://github.com/tvdeyen)) - Add Sprockets manifest file to dummy app [#1632](#1632) ([tvdeyen](https://github.com/tvdeyen)) - Master now tracks 4.4.0.alpha [#1627](#1627) ([tvdeyen](https://github.com/tvdeyen)) - Fix Cell Migration to maintain positions [#1625](#1625) ([mamhoff](https://github.com/mamhoff)) - Cell Upgrader: Match quotation marks in cell name string [#1624](#1624) ([mamhoff](https://github.com/mamhoff)) - Cell Migrator: Maintain element order in fixed elements [#1623](#1623) ([mamhoff](https://github.com/mamhoff)) - Enhance cells upgrader to deal with render_elements from_page: x [#1622](#1622) ([mamhoff](https://github.com/mamhoff))
What is this pull request for?
In several places we now take advantage of Rails eager loading by setting
includes
while loading elements andinverse_of
s on associations, so Active Record knows how to reverse-lookup an object.This dramatically reduces the number of database hits and removes a lot of N+1 especially in the pages and elements API.
This explicitly does not eager load contents and its essence during loading elements from the page in the non-api HTML requests. Why? We cache element views and if we would eager load elements contents and essences we would do this even if we don't need to. This reduces the number of db queries for cached pages, but still produces N+1 for non-cached elements. I think this is a good compromise for the most common case.
Notable changes
In the elements finder I removed the "feature" of loading elements from multiple pages with the same page layout name at once. I doubt this "feature" is actually used by anyone.