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

preserving full stack #304

Open
SyamGadde opened this issue Jun 3, 2022 · 1 comment
Open

preserving full stack #304

SyamGadde opened this issue Jun 3, 2022 · 1 comment
Labels
Can't Fix An issue greenlet cannot work around Feature Request Feature request Question

Comments

@SyamGadde
Copy link

I'm a great fan of py-spy for profiling but have been having issues using it with greenlets because greenlets truncate their Python stack at the frame where the greenlet is created so none of the parent frames are available. I've gotten around this with the following patch for greenlet 1.x versions:

diff --git a/src/greenlet/greenlet.c b/src/greenlet/greenlet.c
index f47bbf8..f429dd6 100644
--- a/src/greenlet/greenlet.c
+++ b/src/greenlet/greenlet.c
@@ -873,7 +873,8 @@ static int GREENLET_NOINLINE(g_initialstub)(void* mark)
     else {
         self->stack_prev = ts_current;
     }
-    self->top_frame = NULL;
+    /* self->top_frame = NULL; */ /* SG */
+    self->top_frame = PyThreadState_GET()->frame; /* SG */
     green_clear_exc(self);
     self->recursion_depth = PyThreadState_GET()->recursion_depth;

Presumably something similar for greenlet 2.x but in greenlet_greenlet.hpp in set_initial_state(). I haven't been able to test on 2.x because I've been unable to get gevent to work with it. Any reasons this approach would be a bad idea?

@jamadden
Copy link
Contributor

Hmm, interesting idea. Though, I'm also a big py-spy user and haven't had any problems using it with greenlets.

I have concerns about whether it's even reliably possible, though, because the frames that created the greenlet can have been exited while the greenlet is still alive. In the best case scenario, we would wind up keeping references to all those frames alive, preventing deallocating any objects referenced by them. That can be a serious problem. (I haven't worked through all the details, but worse scenarios could be anything from confusing/corrupted tracebacks to improper unwinding behaviour [exceptions, try/finally, context managers] or interpreter crashes.)

Here's a contrived example:

import greenlet
import psutil

def func():
    greenlet.getcurrent().parent.switch()

def creates_greenlet():
    l = list(range(500_000))
    g = greenlet.greenlet(func)
    g.switch()
    return g

mem_before = psutil.Process().memory_full_info()
glets = [creates_greenlet() for _ in range(100)]
mem_after = psutil.Process().memory_full_info()

print(mem_before)
print(mem_after)
print('mem delta', (mem_after.uss - mem_before.uss) / 1024 /1024, "MB")

In normal circumstances, this reports that it required 8MB of memory to run, and runs in <4s. If I capture the frame when the greenlet is started, memory usage balloons to nearly 2GB! (And runtime doubles because of GC overhead.)

Even if this were to prove reliably possible, and no one was concerned about keeping all those extra objects around, it would be a substantial behaviour change visible to users. Not just in things like traceback.print_stack() or sys._current_frames(), but I expect things like debuggers to be affected. So it's not something we could just slip in, it would need a major version bump.

@jamadden jamadden added Can't Fix An issue greenlet cannot work around Feature Request Feature request labels Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Can't Fix An issue greenlet cannot work around Feature Request Feature request Question
Projects
None yet
Development

No branches or pull requests

2 participants