Commit 233d285c68e5cb180aa5f96d9e213257ef0d91dc
Exists in
master
and in
4 other branches
Merge pull request #3741 from hiroponz/improve-performance-of-network-graph
Improve performance of network graph
Showing
8 changed files
with
96 additions
and
52 deletions
Show diff stats
app/assets/javascripts/branch-graph.js.coffee
| @@ -9,6 +9,7 @@ class BranchGraph | @@ -9,6 +9,7 @@ class BranchGraph | ||
| 9 | @offsetY = 20 | 9 | @offsetY = 20 |
| 10 | @unitTime = 30 | 10 | @unitTime = 30 |
| 11 | @unitSpace = 10 | 11 | @unitSpace = 10 |
| 12 | + @prev_start = -1 | ||
| 12 | @load() | 13 | @load() |
| 13 | 14 | ||
| 14 | load: -> | 15 | load: -> |
| @@ -24,10 +25,18 @@ class BranchGraph | @@ -24,10 +25,18 @@ class BranchGraph | ||
| 24 | 25 | ||
| 25 | prepareData: (@days, @commits) -> | 26 | prepareData: (@days, @commits) -> |
| 26 | @collectParents() | 27 | @collectParents() |
| 28 | + @graphHeight = $(@element).height() | ||
| 29 | + @graphWidth = $(@element).width() | ||
| 30 | + ch = Math.max(@graphHeight, @offsetY + @unitTime * @mtime + 150) | ||
| 31 | + cw = Math.max(@graphWidth, @offsetX + @unitSpace * @mspace + 300) | ||
| 32 | + @r = Raphael(@element.get(0), cw, ch) | ||
| 33 | + @top = @r.set() | ||
| 34 | + @barHeight = Math.max(@graphHeight, @unitTime * @days.length + 320) | ||
| 27 | 35 | ||
| 28 | for c in @commits | 36 | for c in @commits |
| 29 | c.isParent = true if c.id of @parents | 37 | c.isParent = true if c.id of @parents |
| 30 | @preparedCommits[c.id] = c | 38 | @preparedCommits[c.id] = c |
| 39 | + @markCommit(c) | ||
| 31 | 40 | ||
| 32 | @collectColors() | 41 | @collectColors() |
| 33 | 42 | ||
| @@ -49,18 +58,12 @@ class BranchGraph | @@ -49,18 +58,12 @@ class BranchGraph | ||
| 49 | k++ | 58 | k++ |
| 50 | 59 | ||
| 51 | buildGraph: -> | 60 | buildGraph: -> |
| 52 | - graphHeight = $(@element).height() | ||
| 53 | - graphWidth = $(@element).width() | ||
| 54 | - ch = Math.max(graphHeight, @offsetY + @unitTime * @mtime + 150) | ||
| 55 | - cw = Math.max(graphWidth, @offsetX + @unitSpace * @mspace + 300) | ||
| 56 | - @r = r = Raphael(@element.get(0), cw, ch) | ||
| 57 | - top = r.set() | 61 | + r = @r |
| 58 | cuday = 0 | 62 | cuday = 0 |
| 59 | cumonth = "" | 63 | cumonth = "" |
| 60 | - barHeight = Math.max(graphHeight, @unitTime * @days.length + 320) | ||
| 61 | 64 | ||
| 62 | - r.rect(0, 0, 26, barHeight).attr fill: "#222" | ||
| 63 | - r.rect(26, 0, 20, barHeight).attr fill: "#444" | 65 | + r.rect(0, 0, 26, @barHeight).attr fill: "#222" |
| 66 | + r.rect(26, 0, 20, @barHeight).attr fill: "#444" | ||
| 64 | 67 | ||
| 65 | for day, mm in @days | 68 | for day, mm in @days |
| 66 | if cuday isnt day[0] | 69 | if cuday isnt day[0] |
| @@ -81,42 +84,50 @@ class BranchGraph | @@ -81,42 +84,50 @@ class BranchGraph | ||
| 81 | ) | 84 | ) |
| 82 | cumonth = day[1] | 85 | cumonth = day[1] |
| 83 | 86 | ||
| 84 | - for commit in @commits | ||
| 85 | - x = @offsetX + @unitSpace * (@mspace - commit.space) | ||
| 86 | - y = @offsetY + @unitTime * commit.time | 87 | + @renderPartialGraph() |
| 88 | + | ||
| 89 | + @bindEvents() | ||
| 87 | 90 | ||
| 88 | - @drawDot(x, y, commit) | 91 | + renderPartialGraph: -> |
| 92 | + start = Math.floor((@element.scrollTop() - @offsetY) / @unitTime) - 10 | ||
| 93 | + start = 0 if start < 0 | ||
| 94 | + end = start + 40 | ||
| 95 | + end = @commits.length if @commits.length < end | ||
| 89 | 96 | ||
| 90 | - @drawLines(x, y, commit) | 97 | + if @prev_start == -1 or Math.abs(@prev_start - start) > 10 |
| 98 | + i = start | ||
| 91 | 99 | ||
| 92 | - @appendLabel(x, y, commit.refs) if commit.refs | 100 | + @prev_start = start |
| 93 | 101 | ||
| 94 | - @appendAnchor(top, commit, x, y) | 102 | + while i < end |
| 103 | + commit = @commits[i] | ||
| 104 | + i += 1 | ||
| 95 | 105 | ||
| 96 | - @markCommit(x, y, commit, graphHeight) | 106 | + if commit.hasDrawn isnt true |
| 107 | + x = @offsetX + @unitSpace * (@mspace - commit.space) | ||
| 108 | + y = @offsetY + @unitTime * commit.time | ||
| 97 | 109 | ||
| 98 | - top.toFront() | ||
| 99 | - @bindEvents() | 110 | + @drawDot(x, y, commit) |
| 111 | + | ||
| 112 | + @drawLines(x, y, commit) | ||
| 113 | + | ||
| 114 | + @appendLabel(x, y, commit) | ||
| 115 | + | ||
| 116 | + @appendAnchor(x, y, commit) | ||
| 117 | + | ||
| 118 | + commit.hasDrawn = true | ||
| 119 | + | ||
| 120 | + @top.toFront() | ||
| 100 | 121 | ||
| 101 | bindEvents: -> | 122 | bindEvents: -> |
| 102 | drag = {} | 123 | drag = {} |
| 103 | element = @element | 124 | element = @element |
| 104 | - dragger = (event) -> | ||
| 105 | - element.scrollLeft drag.sl - (event.clientX - drag.x) | ||
| 106 | - element.scrollTop drag.st - (event.clientY - drag.y) | ||
| 107 | - | ||
| 108 | - element.on mousedown: (event) -> | ||
| 109 | - drag = | ||
| 110 | - x: event.clientX | ||
| 111 | - y: event.clientY | ||
| 112 | - st: element.scrollTop() | ||
| 113 | - sl: element.scrollLeft() | ||
| 114 | - $(window).on "mousemove", dragger | 125 | + |
| 126 | + $(element).scroll (event) => | ||
| 127 | + @renderPartialGraph() | ||
| 115 | 128 | ||
| 116 | $(window).on | 129 | $(window).on |
| 117 | - mouseup: -> | ||
| 118 | - $(window).off "mousemove", dragger | ||
| 119 | - keydown: (event) -> | 130 | + keydown: (event) => |
| 120 | # left | 131 | # left |
| 121 | element.scrollLeft element.scrollLeft() - 50 if event.keyCode is 37 | 132 | element.scrollLeft element.scrollLeft() - 50 if event.keyCode is 37 |
| 122 | # top | 133 | # top |
| @@ -125,17 +136,20 @@ class BranchGraph | @@ -125,17 +136,20 @@ class BranchGraph | ||
| 125 | element.scrollLeft element.scrollLeft() + 50 if event.keyCode is 39 | 136 | element.scrollLeft element.scrollLeft() + 50 if event.keyCode is 39 |
| 126 | # bottom | 137 | # bottom |
| 127 | element.scrollTop element.scrollTop() + 50 if event.keyCode is 40 | 138 | element.scrollTop element.scrollTop() + 50 if event.keyCode is 40 |
| 139 | + @renderPartialGraph() | ||
| 140 | + | ||
| 141 | + appendLabel: (x, y, commit) -> | ||
| 142 | + return unless commit.refs | ||
| 128 | 143 | ||
| 129 | - appendLabel: (x, y, refs) -> | ||
| 130 | r = @r | 144 | r = @r |
| 131 | - shortrefs = refs | 145 | + shortrefs = commit.refs |
| 132 | # Truncate if longer than 15 chars | 146 | # Truncate if longer than 15 chars |
| 133 | shortrefs = shortrefs.substr(0, 15) + "…" if shortrefs.length > 17 | 147 | shortrefs = shortrefs.substr(0, 15) + "…" if shortrefs.length > 17 |
| 134 | text = r.text(x + 4, y, shortrefs).attr( | 148 | text = r.text(x + 4, y, shortrefs).attr( |
| 135 | "text-anchor": "start" | 149 | "text-anchor": "start" |
| 136 | font: "10px Monaco, monospace" | 150 | font: "10px Monaco, monospace" |
| 137 | fill: "#FFF" | 151 | fill: "#FFF" |
| 138 | - title: refs | 152 | + title: commit.refs |
| 139 | ) | 153 | ) |
| 140 | textbox = text.getBBox() | 154 | textbox = text.getBBox() |
| 141 | # Create rectangle based on the size of the textbox | 155 | # Create rectangle based on the size of the textbox |
| @@ -156,8 +170,9 @@ class BranchGraph | @@ -156,8 +170,9 @@ class BranchGraph | ||
| 156 | # Set text to front | 170 | # Set text to front |
| 157 | text.toFront() | 171 | text.toFront() |
| 158 | 172 | ||
| 159 | - appendAnchor: (top, commit, x, y) -> | 173 | + appendAnchor: (x, y, commit) -> |
| 160 | r = @r | 174 | r = @r |
| 175 | + top = @top | ||
| 161 | options = @options | 176 | options = @options |
| 162 | anchor = r.circle(x, y, 10).attr( | 177 | anchor = r.circle(x, y, 10).attr( |
| 163 | fill: "#000" | 178 | fill: "#000" |
| @@ -240,16 +255,18 @@ class BranchGraph | @@ -240,16 +255,18 @@ class BranchGraph | ||
| 240 | stroke: color | 255 | stroke: color |
| 241 | "stroke-width": 2) | 256 | "stroke-width": 2) |
| 242 | 257 | ||
| 243 | - markCommit: (x, y, commit, graphHeight) -> | 258 | + markCommit: (commit) -> |
| 244 | if commit.id is @options.commit_id | 259 | if commit.id is @options.commit_id |
| 245 | r = @r | 260 | r = @r |
| 261 | + x = @offsetX + @unitSpace * (@mspace - commit.space) | ||
| 262 | + y = @offsetY + @unitTime * commit.time | ||
| 246 | r.path(["M", x + 5, y, "L", x + 15, y + 4, "L", x + 15, y - 4, "Z"]).attr( | 263 | r.path(["M", x + 5, y, "L", x + 15, y + 4, "L", x + 15, y - 4, "Z"]).attr( |
| 247 | fill: "#000" | 264 | fill: "#000" |
| 248 | "fill-opacity": .5 | 265 | "fill-opacity": .5 |
| 249 | stroke: "none" | 266 | stroke: "none" |
| 250 | ) | 267 | ) |
| 251 | # Displayed in the center | 268 | # Displayed in the center |
| 252 | - @element.scrollTop(y - graphHeight / 2) | 269 | + @element.scrollTop(y - @graphHeight / 2) |
| 253 | 270 | ||
| 254 | Raphael::commitTooltip = (x, y, commit) -> | 271 | Raphael::commitTooltip = (x, y, commit) -> |
| 255 | boxWidth = 300 | 272 | boxWidth = 300 |
app/assets/stylesheets/sections/graph.scss
app/helpers/graph_helper.rb
| @@ -4,8 +4,7 @@ module GraphHelper | @@ -4,8 +4,7 @@ module GraphHelper | ||
| 4 | refs += commit.refs.collect{|r|r.name}.join(" ") if commit.refs | 4 | refs += commit.refs.collect{|r|r.name}.join(" ") if commit.refs |
| 5 | 5 | ||
| 6 | # append note count | 6 | # append note count |
| 7 | - notes = @project.notes.for_commit_id(commit.id) | ||
| 8 | - refs += "[#{notes.count}]" if notes.any? | 7 | + refs += "[#{@graph.notes[commit.id]}]" if @graph.notes[commit.id] > 0 |
| 9 | 8 | ||
| 10 | refs | 9 | refs |
| 11 | end | 10 | end |
app/models/network/graph.rb
| @@ -2,7 +2,7 @@ require "grit" | @@ -2,7 +2,7 @@ require "grit" | ||
| 2 | 2 | ||
| 3 | module Network | 3 | module Network |
| 4 | class Graph | 4 | class Graph |
| 5 | - attr_reader :days, :commits, :map | 5 | + attr_reader :days, :commits, :map, :notes |
| 6 | 6 | ||
| 7 | def self.max_count | 7 | def self.max_count |
| 8 | @max_count ||= 650 | 8 | @max_count ||= 650 |
| @@ -16,10 +16,19 @@ module Network | @@ -16,10 +16,19 @@ module Network | ||
| 16 | 16 | ||
| 17 | @commits = collect_commits | 17 | @commits = collect_commits |
| 18 | @days = index_commits | 18 | @days = index_commits |
| 19 | + @notes = collect_notes | ||
| 19 | end | 20 | end |
| 20 | 21 | ||
| 21 | protected | 22 | protected |
| 22 | 23 | ||
| 24 | + def collect_notes | ||
| 25 | + h = Hash.new(0) | ||
| 26 | + @project.notes.where('noteable_type = ?' ,"Commit").group('notes.commit_id').select('notes.commit_id, count(notes.id) as note_count').each do |item| | ||
| 27 | + h[item["commit_id"]] = item["note_count"] | ||
| 28 | + end | ||
| 29 | + h | ||
| 30 | + end | ||
| 31 | + | ||
| 23 | # Get commits from repository | 32 | # Get commits from repository |
| 24 | # | 33 | # |
| 25 | def collect_commits | 34 | def collect_commits |
| @@ -181,7 +190,7 @@ module Network | @@ -181,7 +190,7 @@ module Network | ||
| 181 | l.spaces << space | 190 | l.spaces << space |
| 182 | # Also add space to parent | 191 | # Also add space to parent |
| 183 | l.parents(@map).each do |parent| | 192 | l.parents(@map).each do |parent| |
| 184 | - if parent.space > 0 | 193 | + if 0 < parent.space && parent.space < space |
| 185 | parent.spaces << space | 194 | parent.spaces << space |
| 186 | end | 195 | end |
| 187 | end | 196 | end |
features/project/network.feature
| @@ -11,14 +11,16 @@ Feature: Project Network Graph | @@ -11,14 +11,16 @@ Feature: Project Network Graph | ||
| 11 | And page should have "master" on graph | 11 | And page should have "master" on graph |
| 12 | 12 | ||
| 13 | @javascript | 13 | @javascript |
| 14 | - Scenario: I should switch ref to "stable" | 14 | + Scenario: I should switch "branch" and "tag" |
| 15 | When I switch ref to "stable" | 15 | When I switch ref to "stable" |
| 16 | - Then page should have network graph | ||
| 17 | - And page should select "stable" in select box | 16 | + Then page should select "stable" in select box |
| 18 | And page should have "stable" on graph | 17 | And page should have "stable" on graph |
| 18 | + When I switch ref to "v2.1.0" | ||
| 19 | + Then page should select "v2.1.0" in select box | ||
| 20 | + And page should have "v2.1.0" on graph | ||
| 19 | 21 | ||
| 20 | @javascript | 22 | @javascript |
| 21 | - Scenario: I should looking for a commit by SHA of "v2.1.0" | 23 | + Scenario: I should looking for a commit by SHA |
| 22 | When I looking for a commit by SHA of "v2.1.0" | 24 | When I looking for a commit by SHA of "v2.1.0" |
| 23 | Then page should have network graph | 25 | Then page should have network graph |
| 24 | And page should select "master" in select box | 26 | And page should select "master" in select box |
features/steps/project/project_network_graph.rb
| @@ -30,10 +30,19 @@ class ProjectNetworkGraph < Spinach::FeatureSteps | @@ -30,10 +30,19 @@ class ProjectNetworkGraph < Spinach::FeatureSteps | ||
| 30 | sleep 2 | 30 | sleep 2 |
| 31 | end | 31 | end |
| 32 | 32 | ||
| 33 | + When 'I switch ref to "v2.1.0"' do | ||
| 34 | + page.select 'v2.1.0', :from => 'ref' | ||
| 35 | + sleep 2 | ||
| 36 | + end | ||
| 37 | + | ||
| 33 | And 'page should select "stable" in select box' do | 38 | And 'page should select "stable" in select box' do |
| 34 | page.should have_selector '#ref_chzn span', :text => "stable" | 39 | page.should have_selector '#ref_chzn span', :text => "stable" |
| 35 | end | 40 | end |
| 36 | 41 | ||
| 42 | + And 'page should select "v2.1.0" in select box' do | ||
| 43 | + page.should have_selector '#ref_chzn span', :text => "v2.1.0" | ||
| 44 | + end | ||
| 45 | + | ||
| 37 | And 'page should have "stable" on graph' do | 46 | And 'page should have "stable" on graph' do |
| 38 | within '.graph' do | 47 | within '.graph' do |
| 39 | page.should have_content 'stable' | 48 | page.should have_content 'stable' |
lib/extracts_path.rb
| @@ -98,9 +98,7 @@ module ExtractsPath | @@ -98,9 +98,7 @@ module ExtractsPath | ||
| 98 | 98 | ||
| 99 | @ref, @path = extract_ref(@id) | 99 | @ref, @path = extract_ref(@id) |
| 100 | 100 | ||
| 101 | - # It is used "@project.repository.commits(@ref, @path, 1, 0)", | ||
| 102 | - # because "@project.repository.commit(@ref)" returns wrong commit when @ref is tag name. | ||
| 103 | - @commit = @project.repository.commits(@ref, @path, 1, 0).first | 101 | + @commit = @project.repository.commit(@ref) |
| 104 | 102 | ||
| 105 | @tree = Tree.new(@project.repository, @commit.id, @ref, @path) | 103 | @tree = Tree.new(@project.repository, @commit.id, @ref, @path) |
| 106 | 104 |
lib/gitlab/git/repository.rb
| @@ -49,7 +49,16 @@ module Gitlab | @@ -49,7 +49,16 @@ module Gitlab | ||
| 49 | 49 | ||
| 50 | def commit(commit_id = nil) | 50 | def commit(commit_id = nil) |
| 51 | commit = if commit_id | 51 | commit = if commit_id |
| 52 | - repo.commit(commit_id) | 52 | + # Find repo.refs first, |
| 53 | + # because if commit_id is "tag name", | ||
| 54 | + # repo.commit(commit_id) returns wrong commit sha | ||
| 55 | + # that is git tag object sha. | ||
| 56 | + ref = repo.refs.find {|r| r.name == commit_id} | ||
| 57 | + if ref | ||
| 58 | + ref.commit | ||
| 59 | + else | ||
| 60 | + repo.commit(commit_id) | ||
| 61 | + end | ||
| 53 | else | 62 | else |
| 54 | repo.commits(root_ref).first | 63 | repo.commits(root_ref).first |
| 55 | end | 64 | end |