Efficiency

Introduce page.rev_ids, and use
that, wherever possible, instead
of page.revisions.

This avoids fetching the text of
all the revisions. D'oh!
This commit is contained in:
Jacques Distler 2011-04-25 10:54:44 -05:00
parent c010e6b7a4
commit 7290e90631
8 changed files with 27 additions and 25 deletions

View file

@ -26,7 +26,7 @@ module CacheSweepingHelper
end
def expire_cached_revisions(page)
page.revisions.count.times do |i|
page.rev_ids.count.times do |i|
revno = i+1
expire_action :controller => 'wiki', :web => page.web.address,
:action => 'revision', :id => page.name, :rev => revno

View file

@ -69,7 +69,7 @@ class WikiController < ApplicationController
def export_html
export_pages_as_zip(html_ext) do |page|
renderer = PageRenderer.new(page.revisions.last)
renderer = PageRenderer.new(page.current_revision)
rendered_page = <<-EOL
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1 plus MathML 2.0 plus SVG 1.1//EN" "http://www.w3.org/2002/04/xhtml-math-svg/xhtml-math-svg-flat.dtd" >
<html xmlns="http://www.w3.org/1999/xhtml">
@ -262,7 +262,7 @@ EOL
redirect_home
end
@link_mode ||= :show
@renderer = PageRenderer.new(@page.revisions.last)
@renderer = PageRenderer.new(@page.current_revision)
# to template
end
@ -276,7 +276,7 @@ EOL
@page ||= wiki.read_page(@web_name, @page_name)
@link_mode ||= :publish
if @page
@renderer = PageRenderer.new(@page.revisions.last)
@renderer = PageRenderer.new(@page.current_revision)
else
real_page = WikiReference.page_that_redirects_for(@web, @page_name)
if real_page
@ -348,7 +348,7 @@ EOL
def show
if @page
begin
@renderer = PageRenderer.new(@page.revisions.last)
@renderer = PageRenderer.new(@page.current_revision)
@show_diff = (params[:mode] == 'diff')
render :action => 'page'
# TODO this rescue should differentiate between errors due to rendering and errors in
@ -383,8 +383,8 @@ EOL
if @page
@revisions_by_day = Hash.new { |h, day| h[day] = [] }
@revision_numbers = Hash.new { |h, id| h[id] = [] }
revision_number = @page.revisions.size
@page.revisions.reverse.each do |rev|
revision_number = @page.rev_ids.size
@page.rev_ids.reverse.each do |rev|
day = Date.new(rev.revised_at.year, rev.revised_at.month, rev.revised_at.day)
@revisions_by_day[day] << rev
@revision_numbers[rev.id] = revision_number
@ -415,7 +415,7 @@ EOL
def s5
if [:markdownMML, :markdownPNG, :markdown].include?(@web.markup)
my_rendered = PageRenderer.new(@page.revisions.last)
my_rendered = PageRenderer.new(@page.current_revision)
@s5_content = my_rendered.display_s5
@s5_theme = my_rendered.s5_theme
else
@ -506,7 +506,7 @@ EOL
if params['rev']
@revision_number = params['rev'].to_i
else
@revision_number = @page.revisions.size
@revision_number = @page.rev_ids.size
end
@revision = @page.revisions[@revision_number - 1]
end

View file

@ -55,7 +55,7 @@ require 'instiki_stringsupport'
# Create a hyperlink to a particular revision of a Wiki page
def link_to_revision(page, revision_number, text = nil, mode = nil, html_options = {})
revision_number == page.revisions.size ?
revision_number == page.rev_ids.size ?
link_to(
text || page.plain_name,
{:web => @web.address, :action => 'show', :id => page.name,
@ -109,7 +109,7 @@ require 'instiki_stringsupport'
end
def rendered_content(page)
PageRenderer.new(page.revisions.last).display_content
PageRenderer.new(page.current_revision).display_content
end
def truncate(text, *args)

View file

@ -6,7 +6,7 @@ module WikiHelper
menu << back_for_revision if @revision_number > 1
menu << current_revision
menu << see_or_hide_changes_for_revision if @revision_number > 1
menu << history if @page.revisions.size > 1
menu << history if @page.rev_ids.size > 1
menu << rollback
menu
end
@ -15,11 +15,11 @@ module WikiHelper
menu = []
menu << edit_page
menu << edit_web if @page.name == "HomePage"
if @page.revisions.size > 1
if @page.rev_ids.size > 1
menu << back_for_page
menu << see_or_hide_changes_for_page
end
menu << history if @page.revisions.size > 1
menu << history if @page.rev_ids.size > 1
menu
end
@ -40,11 +40,11 @@ module WikiHelper
end
def forward
if @revision_number < @page.revisions.size - 1
if @revision_number < @page.rev_ids.size - 1
link_to('Forward in time',
{:web => @web.address, :action => 'revision', :id => @page.name, :rev => @revision_number + 1},
{:class => 'navlink', :accesskey => 'F', :id => 'to_next_revision', :rel => 'nofollow'}) +
" <span class='revisions'>(#{@revision.page.revisions.size - @revision_number} more)</span> ".html_safe
" <span class='revisions'>(#{@revision.page.rev_ids.size - @revision_number} more)</span> ".html_safe
else
link_to('Forward in time', {:web => @web.address, :action => 'show', :id => @page.name},
{:class => 'navlink', :accesskey => 'F', :id => 'to_next_revision', :rel => 'nofollow'}) +
@ -62,9 +62,9 @@ module WikiHelper
def back_for_page
link_to('Back in time',
{:web => @web.address, :action => 'revision', :id => @page.name,
:rev => @page.revisions.size - 1},
:rev => @page.rev_ids.size - 1},
{:class => 'navlink', :accesskey => 'B', :id => 'to_previous_revision', :rel => 'nofollow'}) +
" <span class='revisions'>(#{@page.revisions.size - 1} #{@page.revisions.size - 1 == 1 ? 'revision' : 'revisions'})</span>".html_safe
" <span class='revisions'>(#{@page.rev_ids.size - 1} #{@page.rev_ids.size - 1 == 1 ? 'revision' : 'revisions'})</span>".html_safe
end
def current_revision

View file

@ -1,6 +1,8 @@
class Page < ActiveRecord::Base
belongs_to :web
has_many :revisions, :order => 'id', :dependent => :destroy
#In many cases, we don't need to instantiate the full revisions (with all that textual data)
has_many :rev_ids, :order => 'id', :class_name => 'Revision', :select => 'id, revised_at, page_id, author, ip'
has_many :wiki_references, :order => 'referenced_name'
has_one :current_revision, :class_name => 'Revision', :order => 'id DESC'
@ -9,7 +11,7 @@ class Page < ActiveRecord::Base
end
def revise(content, name, time, author, renderer)
revisions_size = new_record? ? 0 : revisions.size
revisions_size = new_record? ? 0 : rev_ids.size
if (revisions_size > 0) and content == current_revision.content and name == self.name
raise Instiki::ValidationError.new(
"You have tried to save page '#{name}' without changing its content")
@ -46,11 +48,11 @@ class Page < ActiveRecord::Base
end
def revisions?
revisions.size > 1
rev_ids.size > 1
end
def previous_revision(revision)
revision_index = revisions.each_with_index do |rev, index|
revision_index = rev_ids.each_with_index do |rev, index|
if rev.id == revision.id
break index
else

View file

@ -7,7 +7,7 @@
<%- for rev in @revisions_by_day[day] -%>
<li>
<%= link_to_revision(rev.page, @revision_numbers[rev.id],
text= (rev.page.revisions.size == @revision_numbers[rev.id] ?
text= (rev.page.rev_ids.size == @revision_numbers[rev.id] ?
"Current" :
"Revision #{@revision_numbers[rev.id]}" )
) %>

View file

@ -7,7 +7,7 @@
<div id="revision">
<%- if @show_diff -%>
<p class="show_diff">
Showing changes from revision #<%= @page.revisions.size - 1 %> to #<%= @page.revisions.size %>:
Showing changes from revision #<%= @page.rev_ids.size - 1 %> to #<%= @page.rev_ids.size %>:
<ins class="diffins">Added</ins> | <del class="diffdel">Removed</del> | <del class="diffmod">Chan</del><ins class="diffmod">ged</ins>
</p>
<%= @renderer.display_diff %>

View file

@ -8,9 +8,9 @@
<%- for page in @pages_by_day[day] -%>
<li>
<%= link_to_existing_page page %>
<%- if page.revisions.size > 1 %>
<%- if page.rev_ids.size > 1 %>
<span class="views">
( <%= link_to_revision(page, page.revisions.size, text='diff',
( <%= link_to_revision(page, page.rev_ids.size, text='diff',
mode='diff') %> | <%= link_to_history(page, text='history') %> )
</span>
<%- end -%>