Patchwork D4053: [RFC]hgweb: garbage collect on every request in hgweb_mod too

login
register
mail settings
Submitter phabricator
Date Aug. 2, 2018, 2:22 p.m.
Message ID <differential-rev-PHID-DREV-epd773xcuuxp3ar4zrok-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/33107/
State New
Headers show

Comments

phabricator - Aug. 2, 2018, 2:22 p.m.
pulkit created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  We recently updated to mercurial 4.6.2 on our servers and also increased number
  of requests to be made on server at one time. This made hgweb instance takes up
  a lot of memory, sometime upto 50GB of memory when requests are made parallely.
  
  This patch is motivated from
  https://www.mercurial-scm.org/repo/hg-committed/rev/ff2370a70fe8 which adds
  gc.collect() call for the server which serves multiple repositories.
  
  I tried profiling this fix, sometimes this shows less memory usage, sometimes
  more, so I am not sure whether this is right and hence RFC tag.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D4053

AFFECTED FILES
  mercurial/hgweb/hgweb_mod.py

CHANGE DETAILS




To: pulkit, #hg-reviewers
Cc: mercurial-devel
phabricator - Aug. 2, 2018, 5:39 p.m.
av6 added a comment.


  Are you sure you're not just using hgwebdir_mod? hgweb_mod is for cases when you don't use --web-conf flag, at least that seems to be the logic that `hgweb.createapp()` uses to pick one of the two modules.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D4053

To: pulkit, #hg-reviewers
Cc: av6, mercurial-devel
phabricator - Aug. 15, 2018, 11:10 a.m.
pulkit added a comment.


  In https://phab.mercurial-scm.org/D4053#62855, @av6 wrote:
  
  > Are you sure you're not just using hgwebdir_mod? hgweb_mod is for cases when you don't use --web-conf flag, at least that seems to be the logic that `hgweb.createapp()` uses to pick one of the two modules.
  
  
  I checked and we are using hgwebdir_mod. When I was testing locally I was not using --web-conf flag, so maybe that's why I ran into `hgweb_mod`. The problem still exists though and we have limited the number of requests at once. Thanks!

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D4053

To: pulkit, #hg-reviewers
Cc: av6, mercurial-devel

Patch

diff --git a/mercurial/hgweb/hgweb_mod.py b/mercurial/hgweb/hgweb_mod.py
--- a/mercurial/hgweb/hgweb_mod.py
+++ b/mercurial/hgweb/hgweb_mod.py
@@ -305,8 +305,19 @@ 
         with self._obtainrepo() as repo:
             profile = repo.ui.configbool('profiling', 'enabled')
             with profiling.profile(repo.ui, enabled=profile):
-                for r in self._runwsgi(req, res, repo):
-                    yield r
+                try:
+                    for r in self._runwsgi(req, res, repo):
+                        yield r
+                finally:
+                    # There are known cycles in localrepository that prevent
+                    # those objects (and tons of held references) from being
+                    # collected through normal refcounting. We mitigate those
+                    # leaks by performing an explicit GC on every request.
+                    # TODO remove this once leaks are fixed.
+                    # TODO only run this on requests that create localrepository
+                    # instances instead of every request.
+                    gc.collect()
+
 
     def _runwsgi(self, req, res, repo):
         rctx = requestcontext(self, repo, req, res)