#5685 Pagination on SCM log view is inefficient

v1.0.0
closed
General
2015-08-20
2013-01-25
No

Cory Johns
The skip is basically pulling every commit up to the desired page, and then iterating through them until it finds the start of the page
Dave Brondsema
ah. we should fix that
Cory Johns
Problem is that it's hard, maybe impossible, to efficiently convert a skip to a start rev.
Dave Brondsema
oh
guess & iterate?
Cory Johns
Some of the SCMs support it, but only one of the tree
*three
Dave Brondsema
svn?
Cory Johns
I don't recall of the top of my head
Dave Brondsema
seems like since they're linear ints
Cory Johns
Yeah
Dave Brondsema
it'd still help
i'll ticket it up
Cory Johns
Though hg also has support for revisions-to-commits built-in, since the way it does branching is very different
We could certainly make the next paging efficient, since we'll know the last revision. But that won't help for jumping around pages
Oh, it wouldn't work for SVN either, though, because of the path param
You can't know a priori what commits touched a path
Dave Brondsema
ugh
Cory Johns
Hrm. If I added repo_ids to the new LastCommit model (would be nice to have anyway), then we could potentially get a list of all commits that touched a path with a single query. Might not be too inefficient. Then could cache that on the page to do paging with
Would only work for the repos that pre-compute the LCDs, but I'm not entirely convinced that our idea about SVN being too slow to pre-compute stuff on refresh is really valid
It's actually faster than git, in my experience
Since it doesn't have to hit a separate process
And it's especially fast for LCD, since it can pull all the LCD info in one request to SVN instead of having to walk up the tree
But, admittedly, my test case for SVN was smaller than my test case for git

We should also limit the max page size to constrain the runtime of this page. The default option of 250 is probably too high unless we can make a lot of speedups.

Related

Tickets: #5837

Discussion

  • Dave Brondsema

    Dave Brondsema - 2013-01-25
    • Description has changed:

    Diff:

    --- old
    +++ new
    @@ -35,3 +35,6 @@
     Since it doesn't have to hit a separate process
     And it's especially fast for LCD, since it can pull all the LCD info in one request to SVN instead of having to walk up the tree
     But, admittedly, my test case for SVN was smaller than my test case for git
    +
    +
    +We should also limit the max page size to constrain the runtime of this page.  The default option of 250 is probably too high unless we can make a lot of speedups.
    
     
  • Dave Brondsema

    Dave Brondsema - 2013-02-08

    Here's some example timing data where svn_lib.log is high.

    {"time": "2013-02-08 18:12:54,559", "level": "INFO ", "name": "stats", "message": {"url": "/p/dispcalgui/code/1224/log/?page=0", "uptime": 1644, "request_category": "svn", "timings": {"svn_tool.commits": 98, "svn_tool.url_for_commit": 6, "svn_tool.commits_count": 3259, "total": 3789, "mongo": 146, "svn_lib.log": 3348, "svn_tool.__init__": 0, "svn_tool.commit": 3, "ming": 215, "svn_tool.shorthand_for_commit": 1, "socket_write": 0, "socket_read": 104, "svn_tool._oid": 0, "sidebar": 4, "svn_tool._revno": 1, "jinja": 371}}}
    {"time": "2013-02-08 15:34:50,247", "level": "INFO ", "name": "stats", "message": {"url": "/p/peachfuzz/svn/2979/log/?path=", "uptime": 305, "request_category": "svn", "timings": {"svn_tool.commits": 619, "svn_tool.url_for_commit": 6, "svn_tool.commits_count": 69604, "total": 71004, "mongo": 125, "svn_lib.log": 70208, "svn_tool.__init__": 0, "svn_tool.commit": 2, "ming": 654, "svn_tool.shorthand_for_commit": 1, "svn_tool._oid": 1, "sidebar": 7, "svn_tool._revno": 1, "jinja": 727}}}
    
     
  • Dave Brondsema

    Dave Brondsema - 2013-02-08
    • private: No --> Yes
    • milestone: forge-backlog --> forge-feb-22
     
  • Dave Brondsema

    Dave Brondsema - 2013-02-08
    • size: --> 2
     
  • Dave Brondsema

    Dave Brondsema - 2013-02-08

    Pagination isn't really necessary. Can do just next instead by passing the next revision in the link.

     
  • Cory Johns - 2013-02-11
    • status: open --> in-progress
    • assigned_to: Cory Johns
     
  • Cory Johns - 2013-02-12

    allura:cj/5685

    Paging changed, but I'm concerned that the code is loading the full Commit records, then throwing them away and re-loading them, for some repo implementations. That needs to be improved. The performance of the log is still slow, though it won't grow for later "pages" anymore.

     
  • Cory Johns - 2013-02-12
    • status: in-progress --> code-review
     
  • Cory Johns - 2013-02-12

    Change back to calling external git tool pushed as metrics and reimplementation preventing flattening of the entire call history show an order of magnitude improvement. Also applied the change to LastCommit which also shows an order of magnitude improvement. This will need to be done for ForgeHg as well, but the flattening issue needs to be fixed first.

     
  • Cory Johns - 2013-02-12
    • status: code-review --> in-progress
     
  • Cory Johns - 2013-02-13

    allura:cj/5685
    forgehg:cj/5685

     
  • Cory Johns - 2013-02-13
    • status: in-progress --> code-review
     
  • Dave Brondsema

    Dave Brondsema - 2013-02-15
    • qa: Dave Brondsema
     
  • Dave Brondsema

    Dave Brondsema - 2013-02-15
    • status: code-review --> in-progress
     
  • Dave Brondsema

    Dave Brondsema - 2013-02-15

    Looks good for hg & svn. Cory said he'd squash some of the LCD related commits

     
  • Dave Brondsema

    Dave Brondsema - 2013-02-18

    "Older" link should go to the next commit and not repeat the last commit on the current page (although I saw a few cases where it wasn't actually doing that either). That will also let us stop the pagination when we're at the very last commit.

     
  • Cory Johns - 2013-02-19

    allura:cj/5685
    forgehg:cj/5685

    I tried to refactor the affected tests out of ForgeHg and into Allura, but couldn't come up with a way to do so. I could put them in ForgeGit, but that would only be marginally better.

     
  • Cory Johns - 2013-02-19
    • status: in-progress --> code-review
     
  • Dave Brondsema

    Dave Brondsema - 2013-02-19
    • status: code-review --> closed
     
  • Dave Brondsema

    Dave Brondsema - 2013-02-20
    • private: Yes --> No
     

Log in to post a comment.