Skip to content
Snippets Groups Projects
  1. Jun 05, 2021
    • Georges Racinet's avatar
      cext: fix memory leak in phases computation · 9f798c1b
      Georges Racinet authored
      Without this a buffer whose size in bytes is the number of
      changesets in the repository is leaked each time the repository is
      opened and changeset phases are computed.
      
      Impact: the current code in hgwebdir creates a new `localrepository`
      instance for each HTTP request. Since any pull or push is made of several
      requests, a team of 100 people can easily produce thousands of such
      requests per day.
      
      Being a low-level malloc, this leak can't be seen with the gc module and
      tools relying on that, but was spotted by valgrind immediately.
      
      Reproduction
      ------------
      
        for i in range(cl_args.iterations):
            repo = hg.repository(baseui, repo_path)
            rev = repo.revs(rev).first()
            ctx = repo[rev]
      
        del ctx
        del repo
        # avoid any pollution by other type of leak
        # (that should be fixed in 5.8)
        repoview._filteredrepotypes.clear()
      
        gc.collect()
      
      Measurements
      ------------
      
      Resident Set Size (RSS), taken on a clone of
      mozilla-central for performance analysis (440 000
      changesets).
      
      before:
        5.8+hg19.5ac0f2a8ba72  1000 iterations: 1606MB
        5.8+hg19.5ac0f2a8ba72 10000 iterations: 5723MB
      after:
        5.8+hg20.e2084d39e145  1000 iterations:  555MB
        5.8+hg20.e2084d39e145 10000 iterations:  555MB
               (double checked, not a copy/paste error)
      
      (e2084d39e14 is the present changeset, before amendment
      of the message to add the measurements)
      9f798c1b
  2. May 20, 2021
  3. May 04, 2021
  4. May 07, 2021
  5. May 05, 2021
    • Matt Harbison's avatar
      run-tests: fix whitelist/blacklist with directories on Windows · dff19fe2
      Matt Harbison authored
      The file name is resolved with `os.path.relpath()` in the `Test` constructor,
      which yields `\` on Windows.  That doesn't match the `/` separator when using
      MSYS tools to build the list, and it isn't obvious that this is the problem
      because directory separators can mostly be used interchangeably.  The
      `--test-list` argument already seems to be properly handled.
      dff19fe2
  6. Mar 12, 2021
  7. Apr 13, 2021
  8. May 06, 2021
  9. May 12, 2021
    • Matt Harbison's avatar
      util: avoid echoing the password to the console on Windows py3 (issue6446) · 5b351317
      Matt Harbison authored
      The `getpass.getpass()` implementation on Windows first checks if `sys.stdin`
      and `sys.__stdin__` are the same object.  It's not on py3 because the former is
      replaced in dispatch.py with something that doesn't normalize '\n' to '\r\n'.
      When they aren't the same object, it simply calls `sys.stdin.readline()` instead
      of the mscvrt functions that read the input characters before they are echoed.
      
      This simply copies the `getpass.win_getpass()` implementation without the stdin
      check, and byteifies around the edges.  I'm not sure if there's a reasonable
      replacement for the check that we could implement.  When echoing input into the
      hg command, the `ui.interactive()` check causes `ui.getpass()` to bail before
      getting here.  If the proper config switches are used to bypass that and call
      this, the process stalls until '\n' is input into the console.  So there could
      be a deadlock here when run by another command if the wrong config settings are
      applied.
      
      Differential Revision: https://phab.mercurial-scm.org/D10708
      5b351317
  10. May 10, 2021
  11. May 02, 2021
  12. May 01, 2021
    • Matt Harbison's avatar
      tests: stabilize test-persistent-nodemap.t on Windows · 9e3979a2
      Matt Harbison authored
      Several issues here:
      
        - Hooks can't invoke shell scripts on Windows, so use `sh` to launch
        - `dd` in MSYS only recognizes `status=noxfer`
        - The `PATH` updating triggered a massive slowdown, but is no longer needed
      
      I have no idea why, but removing the `PATH` update substantially increased the
      speed of the test.  It was running finishing at ~4:30 with `--debug` and ~14:50
      without it, but now completes in ~2:20 on my Windows laptop.
      
      Differential Revision: https://phab.mercurial-scm.org/D10636
      9e3979a2
  13. May 02, 2021
  14. May 01, 2021
  15. Apr 30, 2021
    • Kyle Lippincott's avatar
      black: make codebase compatible with black v21.4b2 and v20.8b1 · f38bf44e
      Kyle Lippincott authored
      I don't know what exact version of black made it care about these whitespace
      differences, but this is the version I got when I just installed it with
      `pip3 install black`.
      
      I'm intentionally not increasing the version of black required, as I don't want
      to force everyone to upgrade their version of black, and these fixes are
      backwards compatible with black v20.8b1. If there are more issues in the future
      and this becomes a maintenance burden I may do so in a future change.
      
      Tested with both versions of black (I got the older version via
      `pip3 install black==20.8b1`)
      
      Differential Revision: https://phab.mercurial-scm.org/D10539
      f38bf44e
  16. May 04, 2021
  17. May 03, 2021
  18. May 01, 2021
  19. Apr 30, 2021
    • Matt Harbison's avatar
      extensions: ignore exceptions from an extension's `getversion()` method · 55345152
      Matt Harbison authored
      This method is usually called when there's a stacktrace being generated, or with
      `hg version -v`.  Raising another exception risks mangling the bug report info.
      
      I hit this issue when trying to add the method to the keyring extension to
      report the version of the extension and the underlying module, and ran into
      demandimport issues prior to py3.8.  It seems like a wise thing to do anyway,
      though unfortunately there's no convenient `ui` object around to issue a
      warning.  Use 'unknown' to signal that it tried to report a version and failed,
      unlike the default case of printing nothing.
      
      Differential Revision: https://phab.mercurial-scm.org/D10540
      55345152
  20. Apr 28, 2021
    • Matt Harbison's avatar
      git: ensure all dirstate state values are bytes · 9cea55ca
      Matt Harbison authored
      I'm not sure how this particular git status occurs, but after the fallout of
      issue 6510 and getting into the issue 6511 state where `git status` shows the
      files as modified in both the "to be committed" and "not staged" lists,
      `hg diff` was crashing in `workingctx.__contains__()`.
      
      Differential Revision: https://phab.mercurial-scm.org/D10532
      9cea55ca
    • Matt Harbison's avatar
      tests: synchronize the git and Mercurial username · 4c7bc42a
      Matt Harbison authored
      The problem with the default name of "test" set by the test runner is the
      stringutil methods are unable to split out separate user and email addresses
      that git wants.  This means the username is recorded in git as "test <test>".
      Amending a commit with that user ends up trying to use "<test>" as the person
      field for the new commit, and the git library complains about the angle
      brackets.  We should probably abort with a clearer message any time this bad
      form is used with the git extension.
      
      One of the commit dates is tweaked to recreate the ambiguous hash prefix from
      before.
      
      Differential Revision: https://phab.mercurial-scm.org/D10531
      4c7bc42a
  21. Apr 27, 2021
  22. Apr 30, 2021
  23. Apr 20, 2021
  24. Apr 24, 2021
    • Georges Racinet's avatar
      repoview: separate concerns in _filteredrepotypes comment · b7e623ac
      Georges Racinet authored
      The cited issue in Python bugtracker is closed, but hasn't been
      fixed. We've been able to use the attached example and reproduce
      it with Python 3.9.
      
      The point where it turns from needless stress on the GC to
      the an actual leak is when one factors in the fact that the GC
      was before Python 3.4 unable to collect some types (see PEP 442).
      
      Note that even with Python 2.7, the simple example of cycles
      due to __mro__ are collectable. This was seen again with the
      example attached on the CPython issue.
      b7e623ac
  25. Apr 23, 2021
    • Georges Racinet's avatar
      repoview: fix memory leak of filtered repo classes · 76ae43d5
      Georges Racinet authored
      The leak occurs in long-running server processes with
      extensions, and is measured at 110kB per request.
      
      Before this change, the contents of the `_filteredrepotypes`
      cache are not properly garbage collected, despite it begin
      a `WeakKeyDictionary`.
      
      Extensions have a tendency to generate a new repository class
      for each `localrepo` instantiation. Server processes based
      on `hgwebdir_mod` will instantiate a new `localrepo` for each
      HTTP request that involves a repository.
      
      As a result, with a testing process that repeatedly opens a
      repository with several extensions activated
      (`topic` notably among them), we see a steady increase in
      resident memory of 110kB per repository instantiation before this
      change. This is also true, if we call `gc.collect()` at each
      instantiation, like `hgwebdir_mod` does, or not.
      
      The cause of the leak is that the *values* aren't weak references.
      This change uses `weakref.ref` for the values, and this makes
      in our measurements the resident size increase drop to 5kB per
      repository instantiation, with no explicit call of `gc.collect()`
      at all.
      There is currently no reason to believe that this remaining leak
      of 5kB is related to or even due to Mercurial core.
      
      We've also seen evidence that `ui.ui` instances weren't properly
      garbage collected before the change (with the change, they are).
      This could explain why the figures are relatively high.
      
      In theory, the collection of weak references could lead to
      much more misses in the cache, so we measured the impact on
      the original case that was motivation for introducing that cache
      in 7e89bd0cfb86 (see also issue5043): `hg convert` of the
      mozilla-central repository. The bad news here is that there is a
      major memory leak there, both with and without the present changeset.
      There were no more cache misses, and we could see no
      more memory leak with this change: the resident size after importing
      roughly 100000 changesets was at 12.4GB before, and 12.5GB after.
      The small increase is mentioned for completeness only, and we
      believe that it should be ignored, at least as long as the main
      leak isn't fixed. At less than 1% of the main leak, even finding out
      whether it is merely noise would be wasteful.
      
      Original context where this was spotted and first mitigated:
        heptapod/heptapod#466
      
      The leak reduction was also obtained in Heptapod inner HTTP server,
      which amounts to the same as `hgwebdir_mod` for these questions.
      
      The measurements done with Python 3.9, similar figures seen with 3.8.
      More work on our side would be needed to give measurements with 2.7,
      because of testing server process does not support it.
      76ae43d5
  26. Apr 24, 2021
    • Georges Racinet's avatar
      repoview: style change in newtype() cache handling · 42eb8b78
      Georges Racinet authored
      This way of writing it does not change the logic at all,
      but is more fit for the change we want to make in the
      next changeset.
      
      If anything, that's one dict lookup less in the hot path,
      but that should be non measurable.
      42eb8b78
  27. Apr 22, 2021
Loading