classification
Title: Add context manager to temporarily disable GC
Type: enhancement Stage: patch review
Components: Interpreter Core, Library (Lib) Versions: Python 3.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: lisroach Nosy List: eric.snow, gregory.p.smith, josh.r, lisroach, ncoghlan, pablogsal, pitrou, rhettinger, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2017-09-05 23:47 by rhettinger, last changed 2017-11-11 06:04 by ncoghlan.

Pull Requests
URL Status Linked Edit
PR 4224 open pablogsal, 2017-11-02 00:24
Messages (14)
msg301407 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-09-05 23:47
Used like this:

    with gc_disabled():
        run_some_timing()

    with gc_disabled():
        # do_something_that_has_real_time_guarantees
        # such as a pair trade, robotic braking, etc

This would be implemented in C for atomicity and speed.  It would be roughly equivalent to:

    @contextmanager
    def gc_disabled():
        if gc.isenabled():
            gc.disable()
            yield
            gc.enable()
        else:
            yield
msg301633 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-09-07 21:36
+1 from me for the general idea. As far as where to put it goes, I think the `gc` module would be the most appropriate home.
msg301636 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-09-07 21:55
Note that threads can break this. Even without calling gc.enable() explicitly, "with gc_disabled()" in different thread can enable GC inside other "with gc_disabled()" block.
msg301644 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-09-07 22:31
Yes, this will be in the same category as the standard stream redirection context managers - multi-threaded applications either won't be able to use it, or else will need to manage access to it somehow.
msg301645 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2017-09-07 22:43
I believe Raymond is aware of the thread issue.  We discussed it.

If gc.disable() would return the previous state of the gc instead of None and an API to enable based on a passed in bool, both of which are written in C and executed with the GIL (or merely another lock protecting changing the gc state rather than the GIL) held and this Python would works with multiple threads all fighting to control the gc state:

@contextmanager
def gc_disabled():
  previous_enable_state = gc.disable()
  yield
  gc.set_enable(previous_enable_state)

But we don't need to modify gc.disable's return value or add a set_enable() if gc_disabled() itself is not implemented in Python.  That's up to the implementer.
msg301710 - (view) Author: Josh Rosenberg (josh.r) * Date: 2017-09-08 18:02
I'd be -1 on this without a demonstrated broad need for this in at least some context outside of microbenchmarking utilities (which presumably have already implemented similar stuff).

If a minimum bar for applicability isn't applied, we'll end up with dozens of these special purpose managers justified by the last limited applicability one. Stuff like contextlib.closing is justifiable (though I wish it allowed you to replace the method name called, so it could call terminate, kill, release, what-have-you) since cleanup close is so common, but disabling and reenabling gc is usually for timing purposes, and there aren't *that* many tools that need it.

It doesn't seem all that useful for real time purposes either; sure, it disables gc, but it doesn't disable other forms of implicit non-local code execution that are surprisingly hard to predict (e.g. object destruction, including __del__, for non-cyclic cases is going to depend on whether the stuff being decref-ed is still owned outside the block). Disabling GC means a lot in Java, because *all* cleanup is GC; in Python, it's just cycle collection, so you're not giving much in the way of guarantees, as the code in question has to be written with a *lot* of assumptions that Python usually can't support (it's hardly a realtime language).

Seems like if you want a speed up and reliability for this case (and all other context managers intended to be low overhead), it would be better to move parts of contextlib to the C layer (e.g. contextmanager and the classes it's implemented in terms of), so the simple and correct throwaway implementations for arbitrary custom use cases are fast enough; atomicity clearly doesn't actually matter here (it can't be made atomic in any meaningful sense given the lack of thread safety), so any real problem with the provided implementations (assuming they had try/finally added to make them robust) is overhead, not atomicity; the code posted so far would be fine (after adding try/finally) without the speed issues.
msg301720 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-09-08 19:07
This is being proposed for the gc module, not contextlib, so I'm not sure how the previous comment applies.

This is just a convenience API for folks already doing this kind of manipulation themselves.
msg302903 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-09-25 01:58
Assigning this to Lisa for a C implementation, docs, and tests.
msg305402 - (view) Author: Pablo Galindo Salgado (pablogsal) * Date: 2017-11-02 00:26
I have prepared a PR in GitHub with an initial implementation of the context manager trying to fulfil the discussed requirements: https://github.com/python/cpython/pull/3980
msg306053 - (view) Author: Pablo Galindo Salgado (pablogsal) * Date: 2017-11-10 22:30
I just realize that the link I provided is incorrect. This is the correct one (also is the one that appears in this issue anyway):

https://github.com/python/cpython/pull/4224
msg306054 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-11-10 22:46
What is the name of the context manager? gc_disabled, disabled, or Disabled? I see different names in the PR and this confuses me.
msg306071 - (view) Author: Pablo Galindo Salgado (pablogsal) * Date: 2017-11-11 00:18
Sorry about that. The context manager is "gc.Disabled()", which I admit is probably a bad name. The documentation is an example of the "equivalent" Python code as stated by Raymond in the first comment but I see now that this will raise confusion. Please, can you indicate what will be a correct name for the context manager so I can update the PR?
msg306072 - (view) Author: Pablo Galindo Salgado (pablogsal) * Date: 2017-11-11 00:22
Just to clarify the current situation: At this point, the contextmanager is referred as "disabled" in the C code but is exported as "Disabled" to the garbage collection module. The "gc_disabled" is the Python "equivalent" given by Raymond that is included in the documentation
msg306076 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-11-11 06:04
Given the existing "gc.enable", "gc.disable" and "gc.isenabled" APIs, I'd suggest calling this one "gc.ensure_disabled": it ensures the GC is disabled in the with statement body, but only turns it back on at the end if it was previously enabled.

That same name would then be used all the way through the code and documentation.
History
Date User Action Args
2017-11-11 06:04:55ncoghlansetmessages: + msg306076
2017-11-11 00:22:21pablogsalsetmessages: + msg306072
2017-11-11 00:18:45pablogsalsetmessages: + msg306071
2017-11-10 22:46:16serhiy.storchakasetmessages: + msg306054
2017-11-10 22:30:43pablogsalsetmessages: + msg306053
2017-11-02 00:26:33pablogsalsetnosy: + pablogsal
messages: + msg305402
2017-11-02 00:24:53pablogsalsetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request4192
2017-09-25 01:58:28rhettingersetassignee: lisroach

messages: + msg302903
nosy: + lisroach
2017-09-08 19:07:17ncoghlansetmessages: + msg301720
2017-09-08 18:02:46josh.rsetnosy: + josh.r
messages: + msg301710
2017-09-07 22:43:23gregory.p.smithsetmessages: + msg301645
2017-09-07 22:31:50ncoghlansetmessages: + msg301644
2017-09-07 21:55:34serhiy.storchakasetnosy: + pitrou, serhiy.storchaka
messages: + msg301636
2017-09-07 21:36:02ncoghlansetassignee: ncoghlan -> (no value)
messages: + msg301633
components: + Library (Lib)
stage: needs patch
2017-09-05 23:47:23rhettingercreate