Tree performance improvements
Created by: rspeicher
After speeding up the Commits page a bit, I started on the Tree page.
Surprisingly, the single biggest bottleneck was the tree_icon helper. I simplified it, but at the cost of only having two icons: one for folders, one for not-folders. I think it's a worthy tradeoff.
Benchmark code
# Run tree_icon on GitLab's root tree 10 times
require 'benchmark'
require_relative 'config/environment'
p = Project.find_by_code('gitlabhq')
t = p.repo.tree.contents
include TreeHelper
def image_tag(*args)
end
Benchmark.bm do |x|
x.report { 10.times { t.each { |v| tree_icon(v) } } }
endResults:
master 1.590000 24.690000 26.280000 ( 26.290603)
performance 0.000000 0.000000 0.000000 ( 0.000202)Crazy, right?
The rest of the PR is just some refactoring of the tree views. Removes a bunch of logic from them that either wasn't necessary or better fit in a helper.
Finally, I updated Grit to the 2.5.0 official gem (I did it looking for any performance improvements, but we might as well switch to it anyway).
Merge request reports
Activity
Created by: dzaporozhets
Thats why we use own fork https://github.com/gitlabhq/grit/commit/810e3c11787e9d84c5925a7edc3264db0f04bb49
By Administrator on 2012-10-04T08:18:21 (imported from GitLab project)
By Administrator on 2012-10-04T08:18:21 (imported from GitLab)
Created by: dubcanada
Not that I am against pull requesting the official repo, the official repo has 47 pull requests most of which have no comments and the last file change was over 6 months ago.
Even if it was submitted it would probably take a while to ever be included so the need to use the official Grit would still need to exist for a while.
By Administrator on 2012-10-05T17:57:30 (imported from GitLab project)
By Administrator on 2012-10-05T17:57:30 (imported from GitLab)
Created by: vsizov
We are going to migrate to https://github.com/schacon/libgit soon. But it is too much work ;)
By Administrator on 2012-10-08T11:55:03 (imported from GitLab project)
By Administrator on 2012-10-08T11:55:03 (imported from GitLab)
Created by: vsizov
sorry http://libgit2.github.com/
By Administrator on 2012-10-08T11:55:56 (imported from GitLab project)
By Administrator on 2012-10-08T11:55:56 (imported from GitLab)
Created by: dzaporozhets
Anyway for now we'll use our grit fork & this PR should be reviewed/tested and applied. Two icons instead of 3 is ok in compare to increased perfomance
By Administrator on 2012-10-08T12:19:18 (imported from GitLab project)
By Administrator on 2012-10-08T12:19:18 (imported from GitLab)
Created by: SaitoWu
@randx @vsizov we are going to migrate to rugged?
I just want to use grit to improve the MR , no need the satellite any more.
code is just like this: https://github.com/mojombo/grit/blob/master/test/test_git_patching.rb#L21..L38
By Administrator on 2012-10-08T13:04:07 (imported from GitLab project)
By Administrator on 2012-10-08T13:04:07 (imported from GitLab)
Created by: vsizov
I'm currently writing a web editor for GitLab and git_patching really interesting feature for me too. But I don't know, maybe we shouldn't tie on grit specific features. Because someday we will migrate to rugged anyway. Any thoughts?
By Administrator on 2012-10-08T16:34:59 (imported from GitLab project)
By Administrator on 2012-10-08T16:34:59 (imported from GitLab)
Created by: SaitoWu
@vsizov Grit is a git command line wrapper, it use git
write_treecommand to apply a patch. and we need to use acommit_treecommand to commit that tree.If
ruggedhaswrite_treeandcommit_treecommand implementation. I think its ok, not so specific.By Administrator on 2012-10-08T16:25:40 (imported from GitLab project)
By Administrator on 2012-10-08T16:25:40 (imported from GitLab)
Created by: vsizov
Yes. We should benchmark performance of Grit and Rugged as well as do profiling of GitLab to understanding if we really need to migrate.
By Administrator on 2012-10-08T16:46:17 (imported from GitLab project)
By Administrator on 2012-10-08T16:46:17 (imported from GitLab)
Created by: vsizov
@SaitoWu The git_patching requires do clone. But one reason why we've created satellites is that one is extremely fast. Because it is never doing git clone.
By Administrator on 2012-10-10T14:32:15 (imported from GitLab project)
By Administrator on 2012-10-10T14:32:15 (imported from GitLab)
Created by: SaitoWu
@vsizov No, we don't need do a clone.
you can see my test on this gist: https://gist.github.com/3853774
u can use
apply_patchcross two repos. or use it in one.By Administrator on 2012-10-10T14:56:02 (imported from GitLab project)
By Administrator on 2012-10-10T14:56:02 (imported from GitLab)
Created by: vsizov
@SaitoWu but we have bare repo in gitlab! What if I want to create patch directly on the server? For example for web editor.
By Administrator on 2012-10-10T15:38:02 (imported from GitLab project)
By Administrator on 2012-10-10T15:38:02 (imported from GitLab)
Created by: vsizov
@SaitoWu I believe we can use class Tree for that.
By Administrator on 2012-10-10T15:41:15 (imported from GitLab project)
By Administrator on 2012-10-10T15:41:15 (imported from GitLab)
Created by: vsizov
@SaitoWu But i don't know how exactly we should do it. I will think about it.
By Administrator on 2012-10-10T15:46:14 (imported from GitLab project)
By Administrator on 2012-10-10T15:46:14 (imported from GitLab)
Created by: SaitoWu
@vsizov create a patch on server is easy i think.
Just create a new uniq name branch, like
patch-1or whatever. edit and commit it( edit from web, commit by grit.Open a MR.
Master review and merge it.
By Administrator on 2012-10-10T15:51:17 (imported from GitLab project)
By Administrator on 2012-10-10T15:51:17 (imported from GitLab)
