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

Add a frame method to clear expensive details #62134

Closed
pitrou opened this issue May 8, 2013 · 14 comments
Closed

Add a frame method to clear expensive details #62134

pitrou opened this issue May 8, 2013 · 14 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@pitrou
Copy link
Member

pitrou commented May 8, 2013

BPO 17934
Nosy @gvanrossum, @ncoghlan, @pitrou, @vstinner, @florentx, @ericsnowcurrently
Dependencies
  • bpo-18112: PEP 442 implementation
  • Files
  • frame_clear.patch
  • frame_clear2.patch
  • frame_clear_alt.patch
  • frame_clear_alt2.patch
  • frame_clear_alt3.patch
  • 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 2013-08-05.21:32:19.170>
    created_at = <Date 2013-05-08.11:54:02.171>
    labels = ['interpreter-core', 'type-feature']
    title = 'Add a frame method to clear expensive details'
    updated_at = <Date 2013-08-05.21:32:19.169>
    user = 'https://github.com/pitrou'

    bugs.python.org fields:

    activity = <Date 2013-08-05.21:32:19.169>
    actor = 'pitrou'
    assignee = 'none'
    closed = True
    closed_date = <Date 2013-08-05.21:32:19.170>
    closer = 'pitrou'
    components = ['Interpreter Core']
    creation = <Date 2013-05-08.11:54:02.171>
    creator = 'pitrou'
    dependencies = ['18112']
    files = ['30224', '30225', '30244', '30252', '31132']
    hgrepos = []
    issue_num = 17934
    keywords = ['patch']
    message_count = 14.0
    messages = ['188718', '188950', '188952', '188974', '189016', '189026', '189109', '189149', '189151', '189768', '189975', '194214', '194511', '194512']
    nosy_count = 7.0
    nosy_names = ['gvanrossum', 'ncoghlan', 'pitrou', 'vstinner', 'flox', 'python-dev', 'eric.snow']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue17934'
    versions = ['Python 3.4']

    @pitrou
    Copy link
    Member Author

    pitrou commented May 8, 2013

    I think we may want to add a finalize() or close() method on frame objects which would clear all local variables (as well as dereference the globals dict, perhaps), after having optionally run a generator's close() method (if the frame belongs to a generator).

    If I'm not mistaken, it should allow breaking reference cycles, and remove the need for complex traceback processing, which Twisted currently also does: http://twistedmatrix.com/trac/browser/trunk/twisted/python/failure.py#L89

    Note that generator cleanup through the frame has a patch in bpo-17807.

    (spinned off from bpo-17911)

    @pitrou pitrou added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels May 8, 2013
    @pitrou
    Copy link
    Member Author

    pitrou commented May 11, 2013

    Here is a patch. Guido, I would hope this may be useful to you.

    @pitrou
    Copy link
    Member Author

    pitrou commented May 11, 2013

    Updated patch to avoid clearing executing frames.
    Note that the new f_executing member should be re-usable to compute gi_running without storing it.

    @ncoghlan
    Copy link
    Contributor

    Mostly looks good to me, but I think I'd prefer that attempts to clear a running frame raise RuntimeError with an appropriate message.

    I also wonder how this might relate to Eric Snow's proposal to reference the currently executing function from the frame object (see bpo-12857). It seems to me that the "f_func" pointer in that patch could serve the same purpose as the "f_executing" boolean flag in this patch, while providing additional information about the execution context.

    Some other possibly relevant traceback related resource management issues: bpo-6116, bpo-1565525, bpo-9815 (picked up while searching for Eric's RFE above)

    (We may want to add a "clear_frames" convenience method to tracebacks as well)

    @pitrou
    Copy link
    Member Author

    pitrou commented May 12, 2013

    Mostly looks good to me, but I think I'd prefer that attempts to clear
    a running frame raise RuntimeError with an appropriate message.

    Hmm, why not. My intuition was to make frame.clear() a best-effort
    method, but this sounds ok too.

    I also wonder how this might relate to Eric Snow's proposal to
    reference the currently executing function from the frame object (see
    bpo-12857). It seems to me that the "f_func" pointer in that patch
    could serve the same purpose as the "f_executing" boolean flag in this
    patch, while providing additional information about the execution
    context.

    Yes, perhaps. Then Eric's patch can incorporate that change once the
    frame.clear() patch is committed.

    (We may want to add a "clear_frames" convenience method to tracebacks
    as well)

    That, or in the traceback module. The reason I'm proposing this one as a
    frame method is that it can't be done in pure Python.

    @ncoghlan
    Copy link
    Contributor

    +1 to all Antoine's replies :)

    @pitrou
    Copy link
    Member Author

    pitrou commented May 13, 2013

    Here is an alternative patch raising RuntimeError on executing frames.
    Please tell which one you prefer :)

    @pitrou
    Copy link
    Member Author

    pitrou commented May 13, 2013

    Updated patch addressing some of Nick's comments.

    @gvanrossum
    Copy link
    Member

    A downside of using this is that some extended traceback printers (such as cgitb.py in the stdlib, or and some things I've seen in web frameworks) won't be able to print the locals. And pdb's pm() function would lose its value too. So it'll remain a judgment call whether to use this.

    As for Tulip, I think I've broken the cycle I was most concerned about, and I've also gotten rid of the lambda that cost me so much time debugging. :-)

    @pitrou
    Copy link
    Member Author

    pitrou commented May 21, 2013

    I will probably wait for PEP-442 adoption before finalizing this issue.

    @vstinner
    Copy link
    Member

    I may be safer to initialize f->f_executing to 1 at the creation of the frame, and only change its value to 0 when the execution is done. (The name of the attribute may also be changed.) I don't know if it is possible, but a frame should not be cleared just before it is used to execute code.

    @pitrou
    Copy link
    Member Author

    pitrou commented Aug 2, 2013

    Here is an updated patch. Any objections to committing?

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 5, 2013

    New changeset 862ab99ab570 by Antoine Pitrou in branch 'default':
    Issue bpo-17934: Add a clear() method to frame objects, to help clean up expensive details (local variables) and break reference cycles.
    http://hg.python.org/cpython/rev/862ab99ab570

    @pitrou
    Copy link
    Member Author

    pitrou commented Aug 5, 2013

    Ok, I committed the RuntimeError-raising version.

    @pitrou pitrou closed this as completed Aug 5, 2013
    @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
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants