Patchwork [6,of,6] spartan: render changesets server-side on /graph page

login
register
mail settings
Submitter Anton Shestakov
Date Dec. 4, 2017, 12:40 p.m.
Message ID <aa14b97727c79991683a.1512391243@neuro>
Download mbox | patch
Permalink /patch/25922/
State Accepted
Headers show

Comments

Anton Shestakov - Dec. 4, 2017, 12:40 p.m.
# HG changeset patch
# User Anton Shestakov <av6@dwimlabs.net>
# Date 1512385721 -28800
#      Mon Dec 04 19:08:41 2017 +0800
# Node ID aa14b97727c79991683a2ffd45986d18e3657576
# Parent  c85a282fa36ff57c7491700c54a1af9c69ded03c
# EXP-Topic hgweb-more-info
spartan: render changesets server-side on /graph page
Augie Fackler - Dec. 4, 2017, 9:28 p.m.
On Mon, Dec 04, 2017 at 08:40:43PM +0800, Anton Shestakov wrote:
> # HG changeset patch
> # User Anton Shestakov <av6@dwimlabs.net>
> # Date 1512385721 -28800
> #      Mon Dec 04 19:08:41 2017 +0800
> # Node ID aa14b97727c79991683a2ffd45986d18e3657576
> # Parent  c85a282fa36ff57c7491700c54a1af9c69ded03c
> # EXP-Topic hgweb-more-info
> spartan: render changesets server-side on /graph page

queued, thanks
Gregory Szorc - Dec. 5, 2017, 5:46 a.m.
On Mon, Dec 4, 2017 at 1:28 PM, Augie Fackler <raf@durin42.com> wrote:

> On Mon, Dec 04, 2017 at 08:40:43PM +0800, Anton Shestakov wrote:
> > # HG changeset patch
> > # User Anton Shestakov <av6@dwimlabs.net>
> > # Date 1512385721 -28800
> > #      Mon Dec 04 19:08:41 2017 +0800
> > # Node ID aa14b97727c79991683a2ffd45986d18e3657576
> > # Parent  c85a282fa36ff57c7491700c54a1af9c69ded03c
> > # EXP-Topic hgweb-more-info
> > spartan: render changesets server-side on /graph page
>
> queued, thanks
>

Anton,

I'm not sure if it is this series or something before. But the graph view
of hgweb now loads substantially slower for me using my local copy of the
hg repo. Until recently, things would load about as fast as I could scroll
with my mouse wheel. Now there are delays of several hundred milliseconds.
Using `HGPROF=ls hg serve --profile` (the default stat profiler doesn't
work with hgweb), it looks like something is spending a lot of time
calculating visibility. Maybe it has something to do with rendering the
obsolescence state of each changeset?

These changes to hgweb are all great! I don't mean to discount the
improvements. If you need help tracking down the perf issue, I'd be happy
to help.
Anton Shestakov - Dec. 5, 2017, 11:03 a.m.
On Mon, 4 Dec 2017 21:46:55 -0800
Gregory Szorc <gregory.szorc@gmail.com> wrote:

> On Mon, Dec 4, 2017 at 1:28 PM, Augie Fackler <raf@durin42.com> wrote:
> 
> > On Mon, Dec 04, 2017 at 08:40:43PM +0800, Anton Shestakov wrote:
> > > # HG changeset patch
> > > # User Anton Shestakov <av6@dwimlabs.net>
> > > # Date 1512385721 -28800
> > > #      Mon Dec 04 19:08:41 2017 +0800
> > > # Node ID aa14b97727c79991683a2ffd45986d18e3657576
> > > # Parent  c85a282fa36ff57c7491700c54a1af9c69ded03c
> > > # EXP-Topic hgweb-more-info
> > > spartan: render changesets server-side on /graph page
> >
> > queued, thanks
> >
> 
> Anton,
> 
> I'm not sure if it is this series or something before. But the graph view
> of hgweb now loads substantially slower for me using my local copy of the
> hg repo. Until recently, things would load about as fast as I could scroll
> with my mouse wheel. Now there are delays of several hundred milliseconds.
> Using `HGPROF=ls hg serve --profile` (the default stat profiler doesn't
> work with hgweb), it looks like something is spending a lot of time
> calculating visibility. Maybe it has something to do with rendering the
> obsolescence state of each changeset?

This series definitely made hgweb slower, at least in the more
apparent way, simply because it moved the code that renders changesets
to the server side. So not only hgweb needs to produce more HTML, but
the browser needs to download more as well, which I suspect contributes
a sizable amount of time spend on /graph requests.

This is not the root of the problem though, because right now /graph
page doesn't render incrementally as you load more data: everything it
does under the hood when you scroll down to see the next page is to
increment revcount by 60 and redo all the work. So when you scroll
below the initial 60 entries, JS graph code fetches /graph?revcount=120
and replaces the current content with that. Then it goes to 180, 240
and so on. This all is a crude answer to the problem /graph has, AFAIU:
it needs to have graphmod.colored() to pass over all the revs in current
view to produce correct graph. Compare this with /log page which just
loads the next page without changing revcount, which is still as fast
as before phases, obsolescence and instabilities were added.

But not all is lost, obviously, I just need to figure out how to not
re-render and re-download every changeset for /graph if it just needs a
new page.

> These changes to hgweb are all great! I don't mean to discount the
> improvements.

Thanks! It's nice to hear that.

> If you need help tracking down the perf issue, I'd be happy to help.

I have no idea yet where can things be improved, apart from the obvious
problem described above, but having as many eyes on hgweb as possible is
definitely useful.

Patch

diff --git a/contrib/wix/templates.wxs b/contrib/wix/templates.wxs
--- a/contrib/wix/templates.wxs
+++ b/contrib/wix/templates.wxs
@@ -211,6 +211,7 @@ 
             <File Id="spartan.filerevision.tmpl"   Name="filerevision.tmpl" />
             <File Id="spartan.footer.tmpl"         Name="footer.tmpl" />
             <File Id="spartan.graph.tmpl"          Name="graph.tmpl" />
+            <File Id="spartan.graphentry.tmpl"     Name="graphentry.tmpl" />
             <File Id="spartan.header.tmpl"         Name="header.tmpl" />
             <File Id="spartan.index.tmpl"          Name="index.tmpl" />
             <File Id="spartan.manifest.tmpl"       Name="manifest.tmpl" />
diff --git a/mercurial/templates/spartan/graph.tmpl b/mercurial/templates/spartan/graph.tmpl
--- a/mercurial/templates/spartan/graph.tmpl
+++ b/mercurial/templates/spartan/graph.tmpl
@@ -33,7 +33,7 @@  navigate: <small class="navigate">{chang
 <div id="wrapper">
 <ul id="nodebgs"></ul>
 <canvas id="graph" width="{canvaswidth}" height="{canvasheight}"></canvas>
-<ul id="graphnodes"></ul>
+<ul id="graphnodes">{nodes%graphentry}</ul>
 </div>
 
 <script type="text/javascript"{if(nonce, ' nonce="{nonce}"')}>
@@ -52,12 +52,12 @@  graph.vertex = function(x, y, radius, co
 	
 	var bg = '<li class="bg parity' + parity + '"></li>';
 	var left = (this.bg_height - this.box_size) + (this.columns + 1) * this.box_size;
-	var nstyle = 'padding-left: ' + left + 'px;';
-	var item = '<li style="' + nstyle + '"><span class="desc">';
-	item += '<a href="{url|urlescape}rev/' + cur[0] + '{sessionvars%urlparameter}" title="' + cur[0] + '">' + cur[3] + '</a>';
-	item += '</span><span class="info">' + cur[5] + ', by ' + cur[4] + '</span></li>';
+	var item = document.querySelector('[data-node="' + cur.node + '"]');
+	if (item) \{
+		item.style.paddingLeft = left + 'px';
+	}
 
-	return [bg, item];
+	return [bg, ''];
 	
 }
 
diff --git a/mercurial/templates/spartan/graphentry.tmpl b/mercurial/templates/spartan/graphentry.tmpl
new file mode 100644
--- /dev/null
+++ b/mercurial/templates/spartan/graphentry.tmpl
@@ -0,0 +1,6 @@ 
+<li data-node="{node|short}">
+ <span class="desc">
+  <a href="{url|urlescape}rev/{node|short}{sessionvars%urlparameter}">{desc|strip|firstline|escape|nonempty}</a>
+ </span>
+ <span class="info"><span class="age">{date|rfc822date}</span>, by {author|person}</span>
+</li>
diff --git a/mercurial/templates/spartan/map b/mercurial/templates/spartan/map
--- a/mercurial/templates/spartan/map
+++ b/mercurial/templates/spartan/map
@@ -7,6 +7,7 @@  changelog = changelog.tmpl
 shortlog = shortlog.tmpl
 shortlogentry = shortlogentry.tmpl
 graph = graph.tmpl
+graphentry = graphentry.tmpl
 naventry = '<a href="{url|urlescape}log/{node|short}{sessionvars%urlparameter}">{label|escape}</a> '
 navshortentry = '<a href="{url|urlescape}shortlog/{node|short}{sessionvars%urlparameter}">{label|escape}</a> '
 navgraphentry = '<a href="{url|urlescape}graph/{node|short}{sessionvars%urlparameter}">{label|escape}</a> '
diff --git a/tests/test-hgweb-symrev.t b/tests/test-hgweb-symrev.t
--- a/tests/test-hgweb-symrev.t
+++ b/tests/test-hgweb-symrev.t
@@ -946,6 +946,9 @@  Set up the repo
   <a href="/shortlog/tip?style=spartan">shortlog</a>
   <a href="/file/tip/?style=spartan">files</a>
   navigate: <small class="navigate"><a href="/graph/43c799df6e75?style=spartan">(0)</a> <a href="/graph/tip?style=spartan">tip</a> </small>
+    <a href="/rev/9d8c40cba617?style=spartan">third</a>
+    <a href="/rev/a7c1559b7bba?style=spartan">second</a>
+    <a href="/rev/43c799df6e75?style=spartan">first</a>
   navigate: <small class="navigate"><a href="/graph/43c799df6e75?style=spartan">(0)</a> <a href="/graph/tip?style=spartan">tip</a> </small>
 
   $ "$TESTDIR/get-with-headers.py" $LOCALIP:$HGPORT 'tags?style=spartan' | egrep $REVLINKS
@@ -1023,6 +1026,8 @@  Set up the repo
   <a href="/shortlog/xyzzy?style=spartan">shortlog</a>
   <a href="/file/xyzzy/?style=spartan">files</a>
   navigate: <small class="navigate"><a href="/graph/43c799df6e75?style=spartan">(0)</a> <a href="/graph/tip?style=spartan">tip</a> </small>
+    <a href="/rev/a7c1559b7bba?style=spartan">second</a>
+    <a href="/rev/43c799df6e75?style=spartan">first</a>
   navigate: <small class="navigate"><a href="/graph/43c799df6e75?style=spartan">(0)</a> <a href="/graph/tip?style=spartan">tip</a> </small>
 
   $ "$TESTDIR/get-with-headers.py" $LOCALIP:$HGPORT 'file/xyzzy?style=spartan' | egrep $REVLINKS