Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove COUNT_ALLOCS special build #83670

Closed
vstinner opened this issue Jan 29, 2020 · 12 comments
Closed

Remove COUNT_ALLOCS special build #83670

vstinner opened this issue Jan 29, 2020 · 12 comments
Labels
3.9 only security fixes build The build process and cross-build

Comments

@vstinner
Copy link
Member

BPO 39489
Nosy @rhettinger, @vstinner, @serhiy-storchaka
PRs
  • bpo-39489: Remove COUNT_ALLOCS special build #18259
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2020-02-03.14:21:24.624>
    created_at = <Date 2020-01-29.18:16:16.453>
    labels = ['build', '3.9']
    title = 'Remove COUNT_ALLOCS special build'
    updated_at = <Date 2020-02-03.14:21:24.623>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2020-02-03.14:21:24.623>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-02-03.14:21:24.624>
    closer = 'vstinner'
    components = ['Build']
    creation = <Date 2020-01-29.18:16:16.453>
    creator = 'vstinner'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 39489
    keywords = ['patch']
    message_count = 12.0
    messages = ['360978', '360979', '360981', '360985', '360986', '360987', '360988', '361036', '361046', '361051', '361294', '361297']
    nosy_count = 3.0
    nosy_names = ['rhettinger', 'vstinner', 'serhiy.storchaka']
    pr_nums = ['18259']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue39489'
    versions = ['Python 3.9']

    @vstinner
    Copy link
    Member Author

    Python has a COUNT_ALLOCS special build which adds sys.getcounts() function and shows statistics on Python types at exit if -X showalloccount command line option is used.

    I never ever used this feature and I don't know anyone using it.

    But "#ifdef COUNT_ALLOCS" code is scattered all around the code. It requires maintenance. I propose to remove the code to ease maintenance.

    Attached PR shows how much code is requires to support this special build.

    There are now more advanced tools to have similar features:

    • tracemalloc can be used to track memory leaks
    • gc.getobjects() can be called frequently to compute statistics on Python types
    • There are many tools built around gc.getobjects()

    The previous large change related to COUNT_ALLOCS was done in Python 3.6 by bpo-23034:

    "The output of a special Python build with defined COUNT_ALLOCS, SHOW_ALLOC_COUNT or SHOW_TRACK_COUNT macros is now off by default. It can be re-enabled using the -X showalloccount option. It now outputs to stderr instead of stdout. (Contributed by Serhiy Storchaka in bpo-23034.)"

    https://docs.python.org/dev/whatsnew/3.6.html#changes-in-python-command-behavior

    @vstinner vstinner added 3.9 only security fixes build The build process and cross-build labels Jan 29, 2020
    @vstinner
    Copy link
    Member Author

    See also bpo-19527 which discussed COUNT_ALLOCS in 2013.

    COUNT_ALLOCS build is documented in Fedora "DebugPythonStacks" wiki page:
    https://fedoraproject.org/wiki/Features/DebugPythonStacks#Verify_COUNT_ALLOCS

    @rhettinger
    Copy link
    Contributor

    I have never used this feature either.

    @vstinner
    Copy link
    Member Author

    As far as I known, the Fedora package of Python is the most known user of COUNT_ALLOCS special build, and maybe the only user.

    The Fedora package of Python 2.7 builds two binaries:

    • python2.7: release mode, optimized
    • python2.7-debug: debug mode, not optimized, built with COUNT_ALLOCS defined

    Sadly, in practice, python2.7-debug is basically unusable since C extensions providing by Fedora are not compatible: the release and the debug modes have a different ABI. I only fixed this recently in Python 3.8.

    It uses COUNT_ALLOCS since the following patch (package version 2.6.5-10, in 2010) written by Dave Malcolm.

    (1) There is a downstream patch adding --with-count-allocs flag to configure. Patch added to package version 2.6.5-10, in 2010:
    https://src.fedoraproject.org/rpms/python2/c/ab11e4c10f6fef4e2e993ef446953df0f0dbb840

    (2) Another downstream patch adds PYTHONDUMPCOUNTS environment variable to only dump statistics if the variable is set. Patch added to package version 2.7-8, in 2010:
    https://src.fedoraproject.org/rpms/python2/c/5810c5d8b1c876ccc4c547cc71cf20520dd27d85

    => this patch was proposed upstream in bpo-19527. I merged it in Python 2.7.15 (in 2017), but with a different environment variable name: PYTHONSHOWALLOCCOUNT.

    (3) Finally, a 3rd downstream patch fix test_abc when COUNT_ALLOCS is defined. Patch added to package version 2.7.1-1, in 2010:
    https://src.fedoraproject.org/rpms/python2/c/4b97eebe22c8c5db58ae65cdc7e79c3ccd45b0a4

    => Bohuslav "Slavek" Kabrda created bpo-19527 to propose to make multiple test fixes upstream. Serhiy Storchaka modified the proposed patch by adding @test.support.requires_type_collecting decorator. Change merged in 3.5 and default branches, but not in 2.7.

    Another example of Fedora contribution: bpo-31692 reports that test_regrtest fails when Python 2.7.14 is built with COUNT_ALLOCS.

    ---

    Now, the interesting part.

    IMHO COUNT_ALLOCS was basically kept between 2010 and 2015 because "it was there" and nobody asked if it still made sense to use it. It wasn't too expensive to maintain in the Fedora package... until someone asked if it's still worth it to maintain it in 2015.

    The COUNT_ALLOCS macro has been removed from the Fedora package of Python 3 in 2015:

    Extract of the bugzilla, "Reply from Dave Malcolm":
    """
    My hazy recollection is that at the time I was dealing with lots of
    memory leak issues in Python 2, so I was keen to add as much help as
    possible in tracking them down to Python 2 and Python 3.

    I don't think this patch ever really bought us much, and it sounds like
    there are better tools for this now, so feel free to drop the
    COUNT_ALLOC patches.
    """

    It confirms what I wrote in the initial message of this issue: there are now better tool to track Python memory leaks.

    Supporting COUNT_ALLOC in the Fedora package was motivated by Dave Malcolm use case of tracking memory leak, but even Dave wrote: "I don't think this patch ever really bought us much".

    @vstinner
    Copy link
    Member Author

    Python 2.7.15 got a PYTHONSHOWALLOCCOUNT environment variable to dump statistics on types at exit, if Python is built with COUNT_ALLOCS macro defined.

    Example with Fedora python2.7-debug (package python2-debug-2.7.17-1.fc31.x86_64):

    $ PYTHONSHOWALLOCCOUNT=1 python2.7-debug -c pass
    exceptions.ImportError alloc'd: 2, freed: 2, max in use: 1
    symtable entry alloc'd: 3, freed: 3, max in use: 1
    enumerate alloc'd: 2, freed: 2, max in use: 1
    dict alloc'd: 459, freed: 220, max in use: 346
    str alloc'd: 13981, freed: 12142, max in use: 5991
    tuple alloc'd: 5088, freed: 4096, max in use: 2337
    (...)
    fast tuple allocs: 2434, empty: 1753
    fast int allocs: pos: 875, neg: 74
    null strings: 81, 1-strings: 328

    @vstinner
    Copy link
    Member Author

    I looked for "Python COUNT_ALLOCS" on the Internet. This special build seems to be very badly documented on the Internet. Outside Python own documentation, I found almost zero reference to it. IMHO it's basically unused.

    (*) bpo-33058: Two years ago (2018), attempt to make COUNT_ALLOCS build ABI compatible with release build. Extract:

    > Could tracemalloc help you?
    tracemalloc can't distinguish between the usage of gc allocs, normal mallocs, and free list reuse.

    (*) Documentation of all Python builds:

    https://pythonextensionpatterns.readthedocs.io/en/latest/debugging/debug_python.html#debug-version-of-python-count-allocs-label

    @vstinner
    Copy link
    Member Author

    The COUNT_ALLOCS feature itself is quite old. It was added 27 years ago (in 1993):

    commit a9c3c22
    Author: Sjoerd Mullender <sjoerd@acm.org>
    Date: Mon Oct 11 12:54:31 1993 +0000

    * Extended X interface: pixmap objects, colormap objects visual objects,
      image objects, and lots of new methods.
    * Added counting of allocations and deallocations of builtin types if
      COUNT_ALLOCS is defined.  Had to move calls to NEWREF down in some
      files.
    * Bug fix in sorting lists.
    

    @serhiy-storchaka
    Copy link
    Member

    I used COUNT_ALLOCS for curiosity. But I needed slightly different information, so in any case I patched the code.

    AFAIK COUNT_ALLOCS is used in some large projects (maybe Fedora). It was already discussed somewhere on the tracker, but I have no a link.

    @vstinner
    Copy link
    Member Author

    I started a thread on the python-dev mailing list:
    https://mail.python.org/archives/list/python-dev@python.org/thread/YEY2TZIWC6HGMM5Y3QKWKMEBT5Y5C324/

    Serhiy:

    I used COUNT_ALLOCS for curiosity. But I needed slightly different information, so in any case I patched the code.

    Did you use it recently? I understand that the unmodified feature didn't help for your use case. Was it to debug memory leaks?

    Serhiy:

    AFAIK COUNT_ALLOCS is used in some large projects (maybe Fedora). It was already discussed somewhere on the tracker, but I have no a link.

    Did you read previous comments, especially msg360985? It's no longer used in Fedora :-) Technically, it's still used in the python2 package, but it's gone from python3 package. But this issue is about Python 3.9, not Python 2 ;-)

    FYI I'm now working in the Python Maintenance team at Red Hat which maintains python2 and python3 packages of Fedora.

    By the way, Charalampos Stratakis is also in my team and he replied on python-dev:

    "I've never used this feature and it was quite the hassle to maintain those patches downstream, so in my biased opinion, I would welcome the removal."

    https://mail.python.org/archives/list/python-dev@python.org/message/4BI64IXNZMJGXNHOA34UJ35V74IGJNZF/

    @serhiy-storchaka
    Copy link
    Member

    It was many years ago, and I used it to get some statistics about using Python objects of some types. But it did not work as I wanted for cached integers etc. In any case I can patch Python again if I need such information.

    Seems Fedora was the main user of this feature. If it is no longer used in Fedora, I think it can be safely removed.

    @vstinner
    Copy link
    Member Author

    vstinner commented Feb 3, 2020

    New changeset c6e5c11 by Victor Stinner in branch 'master':
    bpo-39489: Remove COUNT_ALLOCS special build (GH-18259)
    c6e5c11

    @vstinner
    Copy link
    Member Author

    vstinner commented Feb 3, 2020

    Done, I remove COUNT_ALLOCS macro and all associated code.

    @vstinner vstinner closed this as completed Feb 3, 2020
    @vstinner vstinner closed this as completed Feb 3, 2020
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 only security fixes build The build process and cross-build
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants