Skip to content

Commit

Permalink
Cache Package#show file list
Browse files Browse the repository at this point in the history
InfluxDB performance measuring showed that this view is a bottleneck on Package#show with
a mean render time of ~ 280 ms (bootstrap) and 160 ms (bentoo). Therefore we
introduce now collection caching.

We also need to introduce timeago_tag jquery gem to be able to cache the table
otherwise the timeago would be cached as well causing wrong results.
  • Loading branch information
ChrisBr committed Dec 12, 2018
1 parent 3f35662 commit 410c581
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 54 deletions.
2 changes: 2 additions & 0 deletions src/api/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ gem 'addressable'
gem 'builder'
# to write the rails metrics directly into InfluxDB.
gem 'influxdb-rails', '>=1.0.0.beta2'
# for client side time ago
gem 'rails-timeago', '~> 2.0'

group :development, :production do
# to have the delayed job daemon
Expand Down
4 changes: 4 additions & 0 deletions src/api/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,9 @@ GEM
nokogiri (>= 1.6)
rails-html-sanitizer (1.0.4)
loofah (~> 2.2, >= 2.2.2)
rails-timeago (2.16.0)
actionpack (>= 3.1)
activesupport (>= 3.1)
rails_tokeninput (1.7.0)
railties (>= 3.1.0)
railties (5.2.2)
Expand Down Expand Up @@ -514,6 +517,7 @@ DEPENDENCIES
pundit
rails (~> 5.2.0)
rails-controller-testing
rails-timeago (~> 2.0)
rails_tokeninput (>= 1.6.1.rc1)
rantly (>= 1.1.0)
rdoc
Expand Down
1 change: 1 addition & 0 deletions src/api/app/assets/javascripts/webui/application.js.erb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
//= require moment
//= require mousetrap
//= require peek
//= require rails-timeago
//
//= require webui/keybindings.js.coffee
//= require webui/application/bento/script.js
Expand Down
1 change: 1 addition & 0 deletions src/api/app/assets/javascripts/webui2/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,4 @@
//= require webui2/package-view_file.js
//= require webui2/staging_workflow.js
//= require webui2/project_monitor.js
//= require rails-timeago
26 changes: 26 additions & 0 deletions src/api/app/views/webui/package/_file.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<tr id="file-<%= valid_xml_id(file[:name]) %>">
<td>
<% link_opts = { action: :view_file, project: project, package: package, filename: file[:name], expand: expand }
unless @is_current_rev
link_opts[:rev] = file[:srcmd5]
end %>
<%= link_to_if(file[:viewable], nbsp(file[:name]), link_opts) %>
</td>
<td><span class="hidden"><%= file[:size].rjust(10, '0') %></span><%= human_readable_fsize(file[:size]) %>
</td>
<td><span class="hidden"><%= file[:mtime] %></span><%= timeago_tag Time.at(file[:mtime].to_i) %>
</td>
<!-- limit download for anonymous user to avoid getting killed by crawlers -->
<td><%= if !User.current.is_nobody? || file[:size].to_i < (4 * 1024 * 1024)
link_to sprite_tag('page_white_put', title: 'Download File'),
file_url(project.name, package.name, file[:name], file[:srcmd5])
end %>
<% if removable_file?(file_name: file[:name], package: package) %>
<% if User.current.can_modify? package %>
<%= link_to sprite_tag('page_white_delete', title: 'Remove File'), {:action => :remove_file, :project => project.to_param,
:package => package.to_param, :filename => file[:name]},
{data: {confirm: "Really remove file '#{file[:name]}'?"}, :method => :post} %>
<% end %>
<% end %>
</td>
</tr>
29 changes: 1 addition & 28 deletions src/api/app/views/webui/package/_files_view.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -9,34 +9,7 @@
</tr>
</thead>
<tbody>
<% @files.each do |file| %>
<tr id="file-<%= valid_xml_id(file[:name]) %>">
<td>
<% link_opts = {action: :view_file, project: @project, package: @package, filename: file[:name], expand: @expand}
unless @is_current_rev
link_opts[:rev] = file[:srcmd5]
end %>
<%= link_to_if(file[:viewable], nbsp(file[:name]), link_opts) %>
</td>
<td><span class="hidden"><%= file[:size].rjust(10, '0') %></span><%= human_readable_fsize(file[:size]) %>
</td>
<td><span class="hidden"><%= file[:mtime] %></span><%= fuzzy_time_string(Time.at(file[:mtime].to_i).to_s) %>
</td>
<!-- limit download for anonymous user to avoid getting killed by crawlers -->
<td><%= if !User.current.is_nobody? || file[:size].to_i < (4 * 1024 * 1024)
link_to sprite_tag('page_white_put', title: 'Download File'),
file_url(@project.name, @package.name, file[:name], file[:srcmd5])
end %>
<% if removable_file?(file_name: file[:name], package: @package) %>
<% if User.current.can_modify? @package %>
<%= link_to sprite_tag('page_white_delete', title: 'Remove File'), {:action => :remove_file, :project => @project.to_param,
:package => @package.to_param, :filename => file[:name]},
{data: {confirm: "Really remove file '#{file[:name]}'?"}, :method => :post} %>
<% end %>
<% end %>
</td>
</tr>
<% end %>
<%= render partial: 'file', collection: @files, cached: true, locals: { project: @project, package: @package, expand: @expand } %>
</tbody>
</table>
<% content_for :ready_function do %>
Expand Down
24 changes: 24 additions & 0 deletions src/api/app/views/webui2/webui/package/_file.html.haml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
- can_be_removed = removable_file?(file_name: file[:name], package: package) && User.current.can_modify?(package)
%tr{ id: "file-#{valid_xml_id(file[:name])}" }
%td.text-word-break-all
- link_opts = { action: :view_file, project: project, package: package, filename: file[:name], expand: expand }
- unless is_current_rev
- link_opts[:rev] = file[:srcmd5]
= link_to_if(file[:viewable], nbsp(file[:name]), link_opts)
%td.text-nowrap
%span.d-none= file[:size].rjust(10, '0')
= human_readable_fsize(file[:size])
%td.text-nowrap
= timeago_tag Time.at(file[:mtime].to_i)
/ limit download for anonymous user to avoid getting killed by crawlers
%td.text-center
- if !User.current.is_nobody? || file[:size].to_i < 4.megabytes
= link_to(file_url(project.name, package.name, file[:name], file[:srcmd5]), title: 'Download file') do
%i.fas.fa-download.text-secondary
- if can_be_removed
= link_to('#', data: { toggle: 'modal', target: "#delete-file-modal-#{file_counter}" },
title: 'Delete file') do
%i.fas.fa-times-circle.text-danger
- if can_be_removed
= render(partial: 'delete_file_dialog',
locals: { project: project.to_param, package: package.to_param, filename: file[:name], file_id: file_counter })
28 changes: 2 additions & 26 deletions src/api/app/views/webui2/webui/package/_files_view.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -8,32 +8,8 @@
%th Changed
%th Actions
%tbody
- files.each_with_index do |file, index|
- can_be_removed = removable_file?(file_name: file[:name], package: package) && User.current.can_modify?(package)
%tr{ id: "file-#{valid_xml_id(file[:name])}" }
%td.text-word-break-all
- link_opts = { action: :view_file, project: project, package: package, filename: file[:name], expand: expand }
- unless is_current_rev
- link_opts[:rev] = file[:srcmd5]
= link_to_if(file[:viewable], nbsp(file[:name]), link_opts)
%td.text-nowrap
%span.d-none= file[:size].rjust(10, '0')
= human_readable_fsize(file[:size])
%td.text-nowrap
%span.d-none= file[:mtime]
= fuzzy_time_string(Time.at(file[:mtime].to_i).to_s)
/ limit download for anonymous user to avoid getting killed by crawlers
%td.text-center
- if !User.current.is_nobody? || file[:size].to_i < 4.megabytes
= link_to(file_url(project.name, package.name, file[:name], file[:srcmd5]), title: 'Download file') do
%i.fas.fa-download.text-secondary
- if can_be_removed
= link_to('#', data: { toggle: 'modal', target: "#delete-file-modal-#{index}" },
title: 'Delete file') do
%i.fas.fa-times-circle.text-danger
- if can_be_removed
= render(partial: 'delete_file_dialog',
locals: { project: project.to_param, package: package.to_param, filename: file[:name], file_id: index })
= render partial: 'file', collection: files, cached: true,
locals: { package: package, project: project, expand: expand, is_current_rev: is_current_rev }
- else
%i This package has no files yet
- if User.current.can_modify?(package)
Expand Down

0 comments on commit 410c581

Please sign in to comment.