Story #2871
Improve workbench performance by using helper methods to cache API lookups during view rendering
100%
Description
- Show new/ready pipeline instance with lots of collections in script_parameters (e.g., GATK exome pipeline): this currently does multiple API lookups for each "select a collection" drop-down.
- Dashboard has a similar problem
- "Metadata" tab on a "show" page does multiple API lookups for each link it displays
- For a common type of lookup like "links pointing to X", create helper methods in ApplicationController to handle preloading and on-demand lookups.
helper_method :links_for_object
def links_for_object object_or_uuid
uuid = object_or_uuid.is_a?(String) ? object_or_uuid : object_or_uuid.uuid
preload_links_for_objects([uuid])
@all_links_for[uuid]
end
helper_method :preload_links_for_object
def preload_links_for_objects objects_and_uuids
uuids = objects_and_uuids.collect { |x| x.is_a?(String) ? x : x.uuid }
@all_links_for ||= {}
if not uuids.select { |x| @all_links_for[x].nil? }.any?
# already preloaded for all of these uuids
return
end
uuids.each do |x|
@all_links_for[x] = []
end
# TODO: make sure we get every page of results from API server
Link.filter([['head_uuid','in',uuids]]).each do |link|
@all_links_for[link.head_uuid] << link
end
end
In any controller/helper/view, when you expect to be looking up links for a set of objects:
preload_links_for_objects @objects
In any controller/helper/view when you need a list of links referencing a given object, instead of doing Link.where(), do:
links_for_object(@object).each do |link|
if link.link_class == 'name'
# do stuff with link.name
end
end
This will at least avoid doing multiple lookups for the same collection ID when rendering N select widgets, etc. Judicious use of "preload" will reduce round-trips further: e.g., combine all of the name/tag lookups for collections#index list into one API call by doing preload_links_for_objects(@objects)
.
Subtasks
Associated revisions
refs #2871
Merge branch '2871-preload-objects'
History
#1
Updated by Radhika Chippada over 6 years ago
- Assigned To set to Radhika Chippada
#2
Updated by Radhika Chippada over 6 years ago
- Story points changed from 2.0 to 3.0
#3
Updated by Tom Clegg over 6 years ago
- Description updated (diff)
#4
Updated by Peter Amstutz over 6 years ago
Pointed workbench at qr1hi and immediately got this error:
#<NoMethodError: undefined method `[]' for nil:NilClass> /home/peter/work/arvados/apps/workbench/app/controllers/application_controller.rb:541:in `block in preload_log_collections_for_objects' /home/peter/work/arvados/apps/workbench/app/controllers/application_controller.rb:539:in `each' /home/peter/work/arvados/apps/workbench/app/controllers/application_controller.rb:539:in `preload_log_collections_for_objects' /home/peter/.rvm/gems/ruby-2.1.1/gems/actionpack-4.1.1/lib/abstract_controller/helpers.rb:70:in `preload_log_collections_for_objects' /home/peter/work/arvados/apps/workbench/app/views/users/_tables.html.erb:42:in `_app_views_users__tables_html_erb___2158683122563325665_36760760' ....
#5
Updated by Peter Amstutz over 6 years ago
times in milliseconds, using the "network" tab of firefox web developer tools.
master¶
dashboard | qr1hi-tpzed-9zdpkpni2yddge6 | 3347 | 3898 | 3877 | 3338 | 2913 |
user | qr1hi-tpzed-9zdpkpni2yddge6 | 10780 | 9677 | 11668 | 11656 | 11195 |
pipeline | qr1hi-d1hrv-pep5f13gwh50paf | 4904 | 3796 | 3496 | 3538 | 3600 |
preload-objects¶
dashboard | qr1hi-tpzed-9zdpkpni2yddge6 | crashes | ||||
user | qr1hi-tpzed-9zdpkpni2yddge6 | 5997 | 4707 | 6015 | 5497 | 5204 |
pipeline | qr1hi-d1hrv-pep5f13gwh50paf | 3985 | 3998 | 5536 | 4402 | 3541 |
40% improvement on metadata in the user page, pipeline components seems like a wash, dashboard crashes.
#6
Updated by Peter Amstutz over 6 years ago
times in milliseconds, using the "network" tab of firefox web developer tools.
master¶
dashboard | qr1hi-tpzed-9zdpkpni2yddge6 | 3289 | 2781 | 3046 | 3357 | 2598 |
user | qr1hi-tpzed-9zdpkpni2yddge6 | 8749 | 9226 | 10466 | 10457 | 9173 |
pipeline | qr1hi-d1hrv-pep5f13gwh50paf | 3221 | 3932 | 3872 | 3194 | 3744 |
preload-objects¶
dashboard | qr1hi-tpzed-9zdpkpni2yddge6 | 2100 | 2354 | 2210 | 1975 | 1960 |
user | qr1hi-tpzed-9zdpkpni2yddge6 | 4650 | 4590 | 4318 | 4004 | 4171 |
pipeline | qr1hi-d1hrv-pep5f13gwh50paf | 3499 | 3231 | 3145 | 3028 | 3328 |
50% reduction in load time for user metadata, 30% reduction in load time for dashboard. Pipeline seems to be within margin of error, no difference.
#7
Updated by Peter Amstutz over 6 years ago
Connecting workbench to 4xphq at revision 799e968
#<NoMethodError: undefined method `friendly_link_name' for nil:NilClass>
/home/peter/work/arvados/apps/workbench/app/helpers/application_helper.rb:95:in `link_to_if_arvados_object'
/home/peter/work/arvados/apps/workbench/app/views/users/_tables.html.erb:148:in `block in app_views_users_tables_html_erb___1317477514263319017_12602200'
/home/peter/work/arvados/apps/workbench/app/views/users/_tables.html.erb:138:in `each'
/home/peter/work/arvados/apps/workbench/app/views/users/_tables.html.erb:138:in `_app_views_users__tables_html_erb___1317477514263319017_12602200'
...
I'm having a hard time reviewing this, I feel like this approach of adding new helper methods and then rewriting the views on a case-by-case basis adds a lot of complexity. A cleaner approach would be to add preloading as a caching layer below the regular query methods so that API used by views doesn't change but for the addition of pre-caching calls by the controller. That said, this does make a measurable improvement in page loading times for dashboard and user metadata, so it is probably worth merging once all the crashes are ironed out.
#8
Updated by Radhika Chippada over 6 years ago
- Status changed from New to Resolved
- % Done changed from 80 to 100
Applied in changeset arvados|commit:85ce092000d72c5dda03bd4763c9613bb9a46437.
closes #2871
Merge branch '2871-preload-objects'