Bug 4258 - memory leak on committing from python
Summary: memory leak on committing from python
Status: RESOLVED FIXED
Alias: None
Product: Mercurial
Classification: Unclassified
Component: Mercurial (show other bugs)
Version: 2.9.2
Hardware: Other Linux
: normal bug
Assignee: Bugzilla
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-21 17:13 UTC by Arne Babenhauserheide
Modified: 2014-07-19 14:17 UTC (History)
6 users (show)

See Also:
Python Version: ---


Attachments
leak.py (716 bytes, text/x-python)
2014-05-21 18:29 UTC, Arne Babenhauserheide
Details
hgrc-reference (25 bytes, application/octet-stream)
2014-05-21 18:29 UTC, Arne Babenhauserheide
Details
objgraph for a leaked filecacheentry (63.66 KB, image/png)
2014-05-21 20:43 UTC, Gregory Szorc
Details
leak.py (1.19 KB, text/x-python)
2014-05-23 04:50 UTC, Arne Babenhauserheide
Details
leak_lockreleasing_memchecking.py (2.57 KB, text/x-python)
2014-05-23 06:32 UTC, Arne Babenhauserheide
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Arne Babenhauserheide 2014-05-21 17:13 UTC
I just started to get out my script for testing the commit performance, but I am stumbling over a strange memory-leak when using mercurial.dispatch.dispatch(req) or mercurial.commands.commit(…)
After 7k commits, I see a memory-consumtion of about 3GiB, and rising.

the part of the script which I think is relevant is here: https://bitbucket.org/ArneBab/hg-vs-git-for-server-apps/src/e57b2a1ca2a427e5f456ac5b6b5f5edc346f6235/run_test.py?at=default#cl-150
this is run in a long loop: https://bitbucket.org/ArneBab/hg-vs-git-for-server-apps/src/e57b2a1ca2a427e5f456ac5b6b5f5edc346f6235/run_test.py?at=default#cl-213

To test it, just clone the repo and execute make. 

https://bitbucket.org/ArneBab/hg-vs-git-for-server-apps

(and watch the memory consumption rise to a few GiB within minutes)
Comment 1 Matt Mackall 2014-05-21 17:41 UTC
Is this new? Can you bisect it?
Comment 2 Arne Babenhauserheide 2014-05-21 18:29 UTC
Created attachment 1767 [details]
leak.py
Comment 3 Arne Babenhauserheide 2014-05-21 18:29 UTC
Created attachment 1768 [details]
hgrc-reference
Comment 4 Arne Babenhauserheide 2014-05-21 18:32 UTC
(In reply to comment #1)
I did not bisect it yet, but I now created a minimal leaking example (see the attachments).

This does not leak as heavily as the performance-scripts, but after 4k commits, it also reaches over 60 MiB for a 4MiB file (with lots of repetiton) and 5.7 MiB of history.
Comment 5 Gregory Szorc 2014-05-21 19:50 UTC
I was able to reproduce the leak with 3.0+303-9fb6f328576a. I hooked up guppy and was able to identify numerous leaks. The largest leaks are coming from str instances. I'm still tracking down exactly from where. We're also leaking filecacheentry and filecachesubentry instances.
Comment 6 Gregory Szorc 2014-05-21 20:42 UTC
I /think/ we may have a cycle at http://selenic.com/repo/hg/file/9fb6f328576a/mercurial/localrepo.py#l863. The filecacheentry instances all chain back to an "onclose" function that chains back to a transaction instance.

Annotate says http://selenic.com/repo/hg/rev/cd443c7589cc is responsible for introducing onclose(). Sure enough, updating to p1(cd443c7589cc) (5dffd06f1e50) makes the leak go away.
Comment 7 Gregory Szorc 2014-05-21 20:43 UTC
Created attachment 1769 [details]
objgraph for a leaked filecacheentry
Comment 8 Gregory Szorc 2014-05-21 21:03 UTC
Patch plugging the leak submitted to list.
Comment 9 Arne Babenhauserheide 2014-05-23 04:50 UTC
Created attachment 1770 [details]
leak.py

A new version of leak.py which more nicely shows that Mercurial with the patch still leaks.

To replicate:

- Run the script. Note the memory usage.
- Run the script again without deleting testrepo/. Notice that the memory usage starts lower than the first ended.

On my system, the first run ends with a memory usage between 29MiB and 31MiB (oscillating).

The second run after 500 commits is still between 23MiB and 24MiB.

I hope that I used the patched mercurial. I only built it in the hg repo with make all and adjusted the paths in the script. I verified that req=request(["version"]); dispatch(req) gives 3.0+5-54d7657d7d1e+20140523).
Comment 10 Arne Babenhauserheide 2014-05-23 06:32 UTC
Created attachment 1771 [details]
leak_lockreleasing_memchecking.py

I created a new version of the leak-script with two major changes:

1) It uses objgraph to check for memory usage.
2) It releases and reaquires the lock just before checking for memory usage.

Disable the lock-release with --norelease. It instantly begins to leak one tuple, one function and one cell per commit.

Also it creates the hgrc-reference, if that does not exist yet, so it is now really a standalone test.
Comment 11 Arne Babenhauserheide 2014-05-23 10:08 UTC
In my performance test, I still see heavy leaking in both versions.

The leakage is even worse when using dispatch.

But when I release and reacquire the log, the objgraph does not show an increase of the number of objects, even though the required memory still rises sharply.

1000 revisions require 170MiB of memory with direct commit and relocking. With dispatch they require about 200MiB (there objgraph shows 1400 additional dicts in 100 revisions).

Example for dispatch object growth in 100 revisions:

dict        12420     +1400
list        25521     +1200
sortdict     4800      +600
config       2400      +300
set          1761      +200
request       800      +100
ui            800      +100
weakref       744        +2
Comment 12 Durham Goode 2014-05-23 13:29 UTC
Arne, when I apply Greg's patch and run your leak_lockreleasing_memchecking.py script, the leak disappears (4000 commits so far with no showgrowth output beyond commit 100).  Are you sure you're running the script against your dev hg and not the system one?
Comment 13 Gregory Szorc 2014-05-23 14:21 UTC
I also started work on a new test to automatically scan for leaks. I'm still working out the details (things like the manifest LRU cache are introducing new objects that get reported as leaks). This will likely turn into a multiple patch series. In the mean time, feel free to experiment. http://hg.stage.mozaws.net/hg/rev/a903ec116060
Comment 14 HG Bot 2014-06-01 19:00 UTC
Fixed by http://selenic.com/repo/hg/rev/99ba1d082287
Gregory Szorc <gregory.szorc@gmail.com>
localrepo: prevent leak of transaction object (issue4258)

The onclose() closure added in cd443c7589cc held a regular reference to
the transaction object. This was causing the transaction to not gc and
a leak to occur.

The closure now holds a reference to the weakref instance and the leak
goes away.

(please test the fix)
Comment 15 Arne Babenhauserheide 2014-06-03 02:46 UTC
(In reply to comment #12)
Hi Durham,

I also see no further show_growth after some time when I release and reaquire the lock, but still the memory consumption rises continually.

I see the same, though to a much larger extend, in my performance test. Within just 2000 commits, the memory consumption rises to 300-400 MiB.
Comment 16 Arne Babenhauserheide 2014-06-03 02:48 UTC
(In reply to comment #13)
Hi Gregory,

I still see the leak with hg 3.0.1. Can you tell me how I can access your experiments? Clicking the link only gets me to an empty page.
Comment 17 Gregory Szorc 2014-06-08 19:07 UTC
The link in comment #13 should be working again. The test doesn't work. But there is code there to run commits in a loop, which should reveal a leak.

I won't have time for a bit to look into this.