Idea #2871
closed
Improve workbench performance by using helper methods to cache API lookups during view rendering
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).
Updated by Radhika Chippada over 10 years ago
- Assigned To set to Radhika Chippada
Updated by Radhika Chippada over 10 years ago
- Story points changed from 2.0 to 3.0
Updated by
Updated by Peter Amstutz over 10 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' ....
Updated by Peter Amstutz over 10 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.
Updated by Peter Amstutz over 10 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.
Updated by Peter Amstutz over 10 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.
Updated by Radhika Chippada over 10 years ago
- Status changed from New to Resolved
- % Done changed from 80 to 100
Applied in changeset arvados|commit:85ce092000d72c5dda03bd4763c9613bb9a46437.