Skip to content
Snippets Groups Projects
Commit 73abd75e authored by Georges Racinet's avatar Georges Racinet
Browse files

load_repo: main cure for memory leak

This brings down the leaking over 1000 requests from
55MB to about 5MB. We are not yet forcing garbage collection
(as `hgwebdir` does).

The `_filteredrepotypes` keeps a correspondence between
unfiltered repository classes and the filtered version.

The problem is that each new repository is an instance of
the single-usage class generated in `hgext3rd.topic.reposetup`,
so these accumulate in `_filteredrepotypes`.

Ironically the mapping seems to be there to avoid some other
case of leaking:

```
# Python <3.4 easily leaks types via __mro__. See
# https://bugs.python.org/issue17950. We cache dynamically created types
# so they won't be leaked on every invocation of repo.filtered().
_filteredrepotypes = weakref.WeakKeyDictionary()
```

One could imagine that garbage collection of the repo would also just
clear its reference from the `WeakKeyDictionary`, but it doesn't
happen.

A final suprise is that this unlocks the collection of `ui` instances.
According to investigation made with `guppy3` the class objects were
actually the ones eating the most memory, but getting rid of `ui`
instances is also good news.
parent e15ae399
No related branches found
No related tags found
2 merge requests!59Mitigate strongly memory leaks related to repository classes,!58server: mono-process mode
......@@ -6,6 +6,7 @@
# SPDX-License-Identifier: GPL-2.0-or-later
import logging
import os
import weakref
from grpc import StatusCode
from mercurial import (
......@@ -13,6 +14,7 @@
hg,
ui as uimod,
)
from mercurial.repoview import _filteredrepotypes
from .stub.shared_pb2 import (
Repository,
)
......@@ -20,6 +22,10 @@
logger = logging.getLogger(__name__)
def clear_repo_class(repo_class):
_filteredrepotypes.pop(repo_class, None)
class HGitalyServicer:
"""Common features of all HGitaly services.
......@@ -75,4 +81,5 @@
context.set_code(StatusCode.NOT_FOUND)
context.set_details(repr(exc.args))
raise KeyError('repo', repo_path)
weakref.finalize(repo, clear_repo_class, repo.unfiltered().__class__)
return repo
......@@ -4,5 +4,6 @@
# GNU General Public License version 2 or any later version.
#
# SPDX-License-Identifier: GPL-2.0-or-later
import gc
import grpc
import pytest
......@@ -7,6 +8,9 @@
import grpc
import pytest
from mercurial import pycompat
from mercurial import (
hg,
pycompat,
)
from heptapod.testhelpers import (
LocalRepoWrapper,
......@@ -57,6 +61,38 @@
assert context.code == grpc.StatusCode.NOT_FOUND
def test_load_repo_gc(tmpdir):
storage_root = tmpdir.join('repos')
storage_root_bytes = pycompat.sysbytes(str(storage_root))
servicer = HGitalyServicer(dict(storname=storage_root_bytes))
# context is used for error raising only
context = FakeContext()
src_path = storage_root.join('awesome-proj.hg')
LocalRepoWrapper.init(src_path)
from mercurial.repoview import _filteredrepotypes as frt
# we're checking that stuff doesn't accumulate in this frt
# since it is a cache, we can start with a clean slate:
frt.clear()
loaded = servicer.load_repo(Repository(storage_name='storname',
relative_path='awesome-proj.hg'),
context)
# not what we really need, but will help demonstrate that proper removal
# from frt actually happens.
assert len(frt) > 0
# let's even make a loop of references:
loaded.itself = loaded
del loaded
gc.collect()
# this may turn out to be optimistic
assert len(frt) == 0
def test_not_found_propagation(grpc_channel, server_repos_root):
# Taking a random RPC to check that the client receives the
# proper error response
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment