#7873 Git branch & tag speedups -- NEEDS INI

v1.3.1
closed
General
2015-06-29
2015-04-17
No

I saw that git pages on forge-allura.apache.org were slow, so I looked at stats.log and saw that the sidebar was the slowest part. I did some additional digging and found 2 specific areas for improvement:

  • in git_main.py change default_branch_name to a @LazyProperty since it is called many times inside a loop in RepositoryApp.sidebar_menu
  • Since sidebar_menu only requests a certain number of branches, pass that "limit" all the way through to git_repo.py's branches method so that is_valid() is only called a minimum number of times needed.
    • make sure the default branch logic to put it at the top of the list still works (e.g. always put it at the top first)

In addition to those changes, generalize and apply the same approach to the tags. And also check ForgeHg to see if mercurial can benefit the same way.

Related

Tickets: #7961

Discussion

<< < 1 2 (Page 2 of 2)
  • Igor Bondarenko - 2015-06-16
    • status: review --> in-progress
     
  • Igor Bondarenko - 2015-06-16

    Looking good!

    Overall timings are much better with caching enabled. First page hit still can be slow, though. What do you think about updating the cache in repo_refresh also, not just clearing it up?

    Also what do you think on including the ref_list = getattr(self._git, field_name) line inside timing measure for cache? Or is it always very fast, so we can ignore that?

    Minor issues:

    • print statements in tests should be removed
    • test failures
    • typo: "resaults" in the development.ini

     
    SF-specific: I guess we need to add repo_refs_cache_threshold = .5 to configtree also.

     

    Last edit: Igor Bondarenko 2015-06-16
  • Heith Seewald - 2015-06-16

    Great notes -- The initial load is much quicker now :)

    Updated @ hs/7873

    As for the configtree... do I just append CONFIGTREE to the ticket name?

     
  • Igor Bondarenko - 2015-06-17
     
  • Igor Bondarenko - 2015-06-17

    I think this is finally done :)

    However I got new test failure

    ======================================================================
    FAIL: forgegit.tests.functional.test_controllers.TestFork.test_merge_request_detail_view
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/var/local/env-allura/lib/python2.7/site-packages/nose-1.3.4-py2.7.egg/nose/case.py", line 197, in runTest
        self.test(*self.arg)
      File "/home/ibondarenko/ibondarenko-1154/forge/ForgeGit/forgegit/tests/functional/test_controllers.py", line 599, in test_merge_request_detail_view
        assert_equal(rev_links[0].get('href'), '/p/test2/code/ci/%s/' % c_id)
    AssertionError: u'/p/test2/code/ci/5f781900aac5e36cc9a48228df2e5c125d8a9765/' != '/p/test2/code/ci/1e146e67985dcd71c74de79613719bef7bddca4a/'
    -------------------- >> begin captured stdout << ---------------------
    

     
    Is caching enabled in tests? I suggest we disable it in tests by default and enable only whenever needed.

    Other than that looks/works good.

    Regarding configtree, I meant we should made changes to the "configtree" repo (/var/local/configtree on a sandbox). I've made the changed and pushed them to ib/7873 already, so don't worry about that.

    AFAIK yes, we should add "NEEDS CONFIGTREE" to the ticket description after we close the ticket, so that it will be immediately visible that ticket requires configuration changes. It helps when we organize notes for the release and also for siteops, I guess.

    You can go ahead and merge this (both allura and configtree), whenever test is fixed.

     

    Last edit: Igor Bondarenko 2015-06-17
    • Dave Brondsema

      Dave Brondsema - 2015-06-17

      Actually I think "NEEDS INI" would be better on the ticket title since "configtree" is an internal thing to sourceforge. (SF people can infer that INI means check configtree)

       
  • Heith Seewald - 2015-06-25
    • summary: Git branch & tag speedups --> Git branch & tag speedups -- NEEDS INI
    • status: in-progress --> closed
     
  • Dave Brondsema

    Dave Brondsema - 2015-06-29
    • labels: performance, sf-current, sf-4 --> performance, sf-4
     
  • Dave Brondsema

    Dave Brondsema - 2015-08-10
    • Milestone: unreleased --> v1.3.1
     
<< < 1 2 (Page 2 of 2)

Log in to post a comment.