(r10) Item11983 < Tasks

NOTE: If you are a developer, please use a private wiki based on foswiki/trunk on a daily base ...or use trunk.foswiki.org to view this page for some minimal testing.
Use Item9693 for docu changes for 1.2 and 2.0.

You are here: Foswiki>Tasks Web>Item11983 (revision 10) (raw view)

A simple view is iterating over ALL revisions of a topic. 

Despite the common believe that META:TOPICINFO is used to cache the latest revision information,
this cache isn't used in most of the cases. 

There's one simple operation of the RCS subsystem requested most frequently ... and implemented most expensively:
getting the latest revision ID. This ID is used to build up a revision history iterator, which basically is a numbered list.

This last rev ID is computed by calling _numRevisions in the store handlers !RcsWrapHandler and !RcsLightHandler.
The _numRevisions function is rather expensive in both implementations forking an external process in the first and reading
all of the ,v file into memory in the second case.

_numRevisions is called 62 times on a normal topic (<nop>System.WebHome). These are 23 calls _numRevisions to different topics and 39 are repetitions.

_numRevisions in turn calls _readTo 1059 times(!RcsLiteHandler).

All of these calls are only done to get the last rev ID, also available in the META:TOPICINFO.

All but one code path doesn't care about META:TOPICINFO. These then trigger a full RCS read.

It seems the !RcsLite code was trying to only read up to a required point: _readTo ... (only read up to the next char required).
However _readTo is wrapped into a while(1) loop in _ensureProcess() which crawls the ,v exhaustively.

_ensureProcessed seems to be invented to make sure all history data has been gathered in case it is required... 

_ensureProcessed is called pretty much all over !RcsLite in different places.
It is a kind of "hit him on the head" just to make sure he is dead: a classic overkill. 

Looking at Foswiki::UI::View, there are two direct calls into the revision system.

<verbatim>
$revIt = $topicObject->getRevisionHistory();
</verbatim>

and the undocumented pseudo-macro =%REVISIONS%=

<verbatim>
     # Show revisions around the one being displayed.
    $tmpl =~ s/%REVISIONS%/
      revisionsAround(
          $session, $topicObject, $requestedRev, $showRev, $maxRev)/e;
</verbatim>

(REVINFOS only works in templates, not in the topic)

The =revIt= isn't used in most of the cases, or only to get the last rev ID, or when a revision was explicitly requested.
When a revision was explicitly requested, =revIt= is traveled down one revision at a time up to the number the revision ID
was found in the history.

The pseudo-macro =%REVISIONS= is used by PatternSkin to display a fragment of the most recent revision history IDs (like r126 < r125 < r124 < r123 for my !SitePreferences topic).
This is implemented by a <strike>view-only private</strike> method called =Foswiki::UI::View::revisionsAround()= also used in =Foswiki::UI::RDiff= which again does a call to =getRevisionsHistory()= on its own (not reusing the one already computed)

and then does a 

<verbatim>
my @revs = $revIt->all();
</verbatim>

Besides =%REVISIONS= being a bit of a usability problem, it yet again triggers a full read of the history.

The entry point for this kind of slowness is _numRevisions being called on various code paths:

   1 while establishing the preference cache, all revisions of all involved topics are read triggered by a =getInfo()=, part of a readTopic(), starting with <nop>System.DefaultPreferences, all topic preferences, site preferences, etc.
   1 readTopic() and thus getInfo() is also triggered by plugins reading parameters from topics, like !AutoViewTemplate, !SmiliesPlugin, !InterwikiPlugin and so on
   1 then comes getRevisionHistory() part of =view=
   1 then comes =%REVISIONS= part of =view=
   1 then comes a series of calls to =%REVINFO= part of the templates, that too trigger a revision system interaction
   1 then come all =%INCLUDEs= (!WebLeftBar, !WebLeftBarLogin, !WebLeftBarWebList, !WebLeftBarExample, !WebTopBarExample, !LanguageSelector

As far as I see META:TOPICINFO is completely ignored by all of these calls resulting in an expensive RCS lookup.

-- Main.MichaelDaum - 05 Jul 2012



Is there any reason at all to have the revision id's ( =r126 < r125 < r124 < r123= ) ever refer back to the actual revisions in rcs, rather than just list current,  -1, -2, -3?  I think it is safe to assume that revisions are always sequential, esp. for this simple list of links.  There is not any tooltip or any other information rendered beyond the numbers that I can find anyway.

-- %USERSWEB%.GeorgeClark - 05 Jul 2012


Well I always found these numbers and links to be a sever problem for people to use. Imho, they have to go out all together and should be replaced with something like this

%IMAGE{"HistorySnap1.png" size="150"}%

available via the "More" drop menu.


-- %USERSWEB%.MichaelDaum - 05 Jul 2012

<verbatim>
[08:38] <MichaelDaum> pharvey said: TOPICINFO line can't be trusted on the .txt
[08:39] <MichaelDaum> you have to. it caches the information deeply buried inside an rcs coffin ... expensive to dig out 
[08:40] <MichaelDaum> ie. as this is done on a massive scale on pages like WebIndex or WebChanges
[08:46] <pharvey> When Foswiki decides the .txt has been externally modified, it doesn't trust the TOPICINFO. That's the current behaviour
[08:56] <MichaelDaum> I had to disable this logic in readTopic
[08:57] <MichaelDaum> the overall logic isnt straight 
[08:58] <MichaelDaum> some code reads TOPICINFO first, some compares it to the "$truth" and tries to rewrite TOPICINFO, a lot of other code paths go the middle way by using the file timestamp to decid
[09:00] <MichaelDaum> IMHO, the last-rev-info should only be read from RCS in some very _very_ pathological cases
[09:00] <MichaelDaum> e.g. when there is no TOPICINFO in the txt file
[09:01] <MichaelDaum> everything else simply is too expensive
[09:02] <MichaelDaum> while comparing file mod time is cheap, the consequences are a significant drop of performance with users not being able to do much about it 
[09:03] <MichaelDaum> the choice is between "cant trust txt" vs "dont touch RCS with a ten foot pole"
[09:04] <MichaelDaum> first means: correctness 4ever, second means performance rulez over correctness
[09:05] <MichaelDaum> the situation immediately changes as soon as reading the last-rev-info record from the revision control system becomes as cheap as opening up the txt file
[09:06] <MichaelDaum> like cahce it in a memcached
[09:07] <MichaelDaum> or just plain memory for modperlers or fastcgieans
</verbatim>

-- Main.MichaelDaum - 08 Jul 2012

Here's a [[%ATTACHURL%/performance.patch][hotfix for foswiki-1.1.5 to get it back to performance]]

-- Main.MichaelDaum - 12 Jul 2012




just a quick note - I think this change is broken. see http://trunk.foswiki.org/System/WebChanges

-- %USERSWEB%.SvenDowideit - 15 Jul 2012
%COMMENT%

ItemTemplate edit

Summary	prevent excessive calls into the revision system
ReportedBy	MichaelDaum
Codebase	1.1.5, trunk
SVN Range
AppliesTo	Engine
Component
Priority	Urgent
CurrentState	New
WaitingFor
Checkins	Foswikirev:15108 Foswikirev:15109 Foswikirev:15118
TargetRelease	minor
ReleasedIn	1.2.0
CheckinsOnBranches	trunk
trunkCheckins	Foswikirev:15108 Foswikirev:15109 Foswikirev:15118
Release01x01Checkins