Restructure new findaproject page for #4129 #4142
Conversation
Gemfile
Outdated
| gem 'memory_profiler' | ||
| gem 'meta_request' | ||
| gem 'rack-mini-profiler', require: false | ||
| gem 'stackprof' |
There was a problem hiding this comment.
These are dev tools so I moved it here. We can move it back in case it is being used in prod>
| unless session[:guest_user_id].nil? | ||
| User.where(id: session[:guest_user_id]).first | ||
| end | ||
| User.find_by(id: session[:guest_user_id]) |
There was a problem hiding this comment.
Exact same query in SQL, just shorter way of writing it. Will give nil if not present, so same function.
| .includes({ random_collections: :tags }, { random_document_sets: { collection: :tags } }) | ||
|
|
||
| @org_owners = users.findaproject_orgs.order(:display_name).distinct | ||
| @individual_owners = users.findaproject_individuals.order(:display_name).distinct |
There was a problem hiding this comment.
These will be subqueries from users
| .includes(:owner, { next_untranscribed_page: :work }) | ||
| .where(owner_user_id: @org_owners.select(:id) + @individual_owners.select(:id)).sample(5) | ||
| colls = Collection.carousel | ||
| .includes(:owner, { next_untranscribed_page: :work }) |
There was a problem hiding this comment.
N+1 issue here. Checking the html we access next_untranscribed_page, and then it's work. Both of these have to be preloaded.
Also it seems, as long as we preload correct relationships, turning it to array afterwards will not cause more query
| collections_query = Collection.search(search_key).unrestricted.includes(:owner) | ||
| document_sets_query = DocumentSet.search(search_key).unrestricted.includes(:owner) | ||
|
|
||
| collections_query + document_sets_query |
There was a problem hiding this comment.
Again, we access owner from search_results so preload that too.
| users = User.owners | ||
| .joins(:collections) | ||
| .left_outer_joins(:document_sets) | ||
| .includes({ random_collections: :tags }, { random_document_sets: { collection: :tags } }) |
There was a problem hiding this comment.
This is a bit tricky. So we cannot preload queries with dynamic like random.
So what I did here is remove the random_sample scope in random_collections, then we sample later via array to SQL calls.
There was a problem hiding this comment.
Although, in hindsight, this makes 'random_collection' and 'random_document_sets' scope feel like incorrect labels now.
| -[@org_owners, @individual_owners].each do |owners| | ||
| -owners.each do |owner| | ||
| -projects = (owner.random_collections+owner.random_document_sets) | ||
| -projects = (owner.random_collections.sample(5) + owner.random_document_sets.sample(5)) |
There was a problem hiding this comment.
We sample here as we can't preload with dynamic random query
| @@ -59,12 +62,12 @@ | |||
| -if owner.about.present? | |||
| .description | |||
| =owner.about | |||
| -tag_map = projects.map{|p| p.tags.canonical}.flatten.group_by{|t| t}.map{|k,v| [k.ai_text, v.size]}.sort_by{|k,v| v}.reverse | |||
| -tag_map = projects.flat_map(&:tags).group_by(&:canonical).map { |tag, tag_list| [tag.ai_text, tag_list.size] }.sort_by(&:last).reverse | |||
There was a problem hiding this comment.
I am not 100% sure if this functions exactly how you intended it to be (so please correct me if I'm wrong here). Though based on my testing my approach produced less SQL queries (so no n+1 it seems). Not 100% sure
|
All in all, in my local dev I was able to reduce from 150+ queries to 49-ish |
Closes #4129