DmitryTsepelev / ar_lazy_preload

Lazy loading associations for the ActiveRecord models
MIT License
677 stars 24 forks source link

Fix deep association preloading when specified via array of hash #14

Closed PikachuEXE closed 5 years ago

PikachuEXE commented 5 years ago

The last obvious bug I found

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 132


Totals Coverage Status
Change from base Build 129: 0.0%
Covered Lines: 510
Relevant Lines: 510

💛 - Coveralls
PikachuEXE commented 5 years ago

Force pushed to fix code coverage

DmitryTsepelev commented 5 years ago

Thanks for the PR! Merging it and releasing a new version in a second!

Just curious, are you using the gem in the prod app? :)

PikachuEXE commented 5 years ago

Not yet I am testing with real project though (which is why I found these bugs)

PikachuEXE commented 5 years ago

Using the gem on prod now But ArLazyPreload.config.auto_preload is still false at the moment Turning it on next week

DmitryTsepelev commented 5 years ago

@PikachuEXE Amazing! Please let me know if you find something else

PikachuEXE commented 5 years ago

I found that the response time becomes very high after enabling it screen shot 2018-11-26 at 3 53 46 pm

Let me try to reproduce it locally Not sure how to trace what's using so much time though

DmitryTsepelev commented 5 years ago

Interesting! Is there a chance that the initial collection contains a huge amount of records? For instance, if we load 1000 customers from the DB and each customer has 100 orders, User.all.first.orders would cause 1000 * 100 initialized AR objects in memory

PikachuEXE commented 5 years ago

I got this template: (rabl)

object false

child @city => :data do
  attributes :name

  node :latitude do |city|
    display_baidu_maps? ? city.latitude_bd09 : city.latitude
  end
  node :longitude do |city|
    display_baidu_maps? ? city.longitude_bd09 : city.longitude
  end

  # Used later for default value
  empty_array = Array.new.freeze

  # neighbourhood_id presence means getting low level markers one by one
  neighbourhoods_requested               = @p.fetch(:neighbourhoods_requested)
  buildings_grouped_by_neighbourhood_ids = @p.fetch(:buildings_grouped_by_neighbourhood_ids)
  listing_ids_grouped_by_building_ids    = @p.fetch(:listing_ids_grouped_by_building_ids)
  recently_posted_listing_counts_by_building_ids =
    @p.fetch(:recently_posted_listing_counts_by_building_ids)

  node :neighbourhoods do
    neighbourhoods_requested.map do |n|
      # The default value is a shared frozen array to reduce objects created
      # There should be no write operation
      buildings_in_neighbourhood =
        buildings_grouped_by_neighbourhood_ids.
        fetch(n.id) { empty_array }

      buildings_node_data = buildings_in_neighbourhood.map do |b|
        listing_ids = listing_ids_grouped_by_building_ids.
          fetch(b.id) { empty_array }

        {
          name: b.name,

          latitude:   display_baidu_maps? ? b.latitude_bd09 : b.latitude,
          longitude:  display_baidu_maps? ? b.longitude_bd09 : b.longitude,

          has_recently_posted_listings:
            recently_posted_listing_counts_by_building_ids.key?(b.id),

          listing_ids: listing_ids,
        }
      end

      {
        id:   n.id,
        name: n.name,

        latitude:   n.latitude_for_map_center(coord_system_id: (display_baidu_maps? ? :bd09 : :wgs84)),
        longitude:  n.longitude_for_map_center(coord_system_id: (display_baidu_maps? ? :bd09 : :wgs84)),

        buildings: buildings_node_data,
      }
    end
  end
end

and this is part of the controller (which fetches the data):

# Assume 100-200ish for count
filtered_listings = ::Listing.where(a_few_or_many_criteria: :without_includes_nor_lazy_preload)

filtered_neighbourhood_ids = valid_neighbourhood_ids_for_map_data
filtered_neighbourhoods =
  Neighbourhood.
    with_these_ids_only(filtered_neighbourhood_ids)
neighbourhoods_requested = filtered_neighbourhoods.
  lazy_preload(
    :translations,
  ).
  load
@p[:neighbourhoods_requested] = neighbourhoods_requested

listings_in_neighbourhoods = filtered_listings.
  in_neighbourhood_only(neighbourhoods_requested)
listings_grouped_by_building_ids = listings_in_neighbourhoods.
  select(:building_id, :id).to_a.
  group_by(&:building_id)
listing_ids_grouped_by_building_ids = Hash[
  listings_grouped_by_building_ids.map do |b_id, listings|
    [b_id, listings.map(&:id)]
  end
]
@p[:listing_ids_grouped_by_building_ids] =
  listing_ids_grouped_by_building_ids

buildings_linked_with_neighbourhoods =
  ::Building.
    in_neighbourhood_only(filtered_neighbourhood_ids)

buildings_linked_with_n_and_listings = if page_is_for_rental
  buildings_linked_with_neighbourhoods.
    with_available_for_search_for_rent_listings_only
else
  buildings_linked_with_neighbourhoods.
    with_available_for_search_for_sale_listings_only
end

buildings_grouped_by_neighbourhood_ids =
  buildings_linked_with_n_and_listings.
    lazy_preload(
      :housing_estate_parent_building,
      :translations,
    ).
    to_a.
    group_by(&:neighbourhood_id)
@p[:buildings_grouped_by_neighbourhood_ids] =
  buildings_grouped_by_neighbourhood_ids

But still I got no idea what's causing the slowness yet