Hi Robert, thanks for the feedback, I'll use it to write up a CEP for this. It looks like there's still room for discussion, and this isn't a trivial/obvious thing to implement, so it's worth some planning.
Robert Bradshaw, 13.11.2009 07:47: > On Nov 12, 2009, at 7:08 PM, Stefan Behnel wrote: > >> Hi all, >> >> warning: long e-mail ahead, don't read in a hurry! >> >> I gave generator functions a couple of thoughts. Implementing them >> actually >> sounds simpler than it is, not because of the state keeping, but >> because of >> the refactoring (point 1 below) that I would like to see done before >> going >> there. >> >> Here's what I think should be done: >> >> 1) refactor def functions into a Python wrapper and a static C >> function >> * Python wrapper does all argument unpacking, return value packing >> and >> the final exception propagation >> * C function contains the complete body of the original function and >> returns the return value directly > > That could clean things up a lot--it would be nice if this refactoring > cleaned up some of the redundancies and inconsistencies between > CFuncDefNode and DefNode. Hopefully, yes. > I'm not sure exactly what you mean by "final > exception propagation" or "return value packing" though, as we still > want cdef functions to propagate the exception. Right, I guess I got confused here. The return type of a def function is always 'object', so that will be inherited by the underlying C function. > This could make cpdef > functions more natural as well. I think the extra c function call > overhead should be negligible, but we should be sure. Perhaps I'm > prematurely optimizing, but I hate the idea of potentially introducing > regressions. Given that the only case where this matters is when the function is called from Python code, including all the Python call overhead, I honestly doubt that the overhead of a single predictable C function call is even measurable. In the case of very short functions (where the call overhead starts becoming interesting), the C compiler might even end up inlining the C function, so you'd loose the overhead completely. >> b) closure functions: >> - C function has METH_NOARGS signature I actually didn't quite mean METH_NOARGS but just a single object in the signature (maybe even typed as the specific closure type). >> - Python wrapper creates closure and fills in arguments >> - Python wrapper calls C function with closure as 'self' > > I'm not sure I'm following you here--the closure is created when the > function is created, not when it's called, and the function needs to > take its arguments later. True. Sorry for the noise. So we actually need to pass the closure *and* the parameters. No real change here compared to the current workings. > On that note, I certainly want to support > > cdef add_n(n): > def f(x): > return x+n > return f That shouldn't be hard to do, as only f needs a closure here. > and perhaps, if we can figure out a clean way to do it, > > cdef ??? add_n(n): > cdef int f(int x): > return x+n > return f > > though this last case is not as important. It's certainly less important. Although it shouldn't be impossible to do either. "f" could become a C method in the closure object. But the return value would be less obvious, yes. Maybe something similar to the buffer protocol would work here, which transparently unpacks an object into a C value (a function pointer in this case). But the result would have to be a Python object in any case, so maybe we could let it implement __call__, and then provide a builtin type declaration for the value that would allow callers to bypass __call__ and instead call the underlying C method directly. But that's certainly a future add-on, nothing to care about now. >> 2) support writing utility code in Cython (does this work already?) >> * likely just compile TreeFragments inside of the utility_scope? >> (does the utility_scope actually have a unique mangling prefix >> or will it interfere with a user provided "utility" module?) >> >> 3) implement a generic 'generator' type in Cython code (see code >> below) >> * methods: __iter__, __next__, send, throw, close (as in PEP 342, >> see >> http://www.python.org/dev/peps/pep-0342/ ) >> * fields: closure, exception, __weakref__, C function pointer > > I assume the motivation is that this would be easier than just > generating a class with these methods every time? There's overhead to > calling virtual methods, and it seems odd to direct throw() and > __next__/send() into the same method, only to have an if statement to > separate them in its body. No, the if statement must be generated for each yield, so that the exception originates from the correct source line and takes the correct exception handling path. Nothing keeps you from putting a yield into a try-except or try-finally block, for example. So there needs to be some support in the yield node, which basically raises the exception. I just set the exception values outside of the generator function to avoid having to pass them into the function as arguments. >> 4) implement generators as extension to 1b) >> * Python wrapper works mostly as in 1b), but >> - does not call the C function >> - creates and returns a generator instance instead and fills in >> the >> created closure and the pointer to the C function part of the >> generator function > > I was imagining > > def my_xrange(n): > i = 0 > while i < n: > yield i > i += 1 > > would get transformed into > > def my_xrange(n): > return __Pyx_my_xrange_generator_class(n) Yes, I got confused again. So the original generator function itself will just get replaced by a dumb function that lives completely without closures. >> * generator functions become modified closure functions: >> - METH_O signature instead of METH_NOARGS to receive the send(x) >> value >> directly (note that gen.__next__() is defined as >> gen.send(None) and >> gen.throw(exc) could be implemented as gen.send(NULL)) >> - closures additionally contain function temps (I'm thinking of a >> union of structs, i.e. one struct for each set of temps that >> existed >> during the code generation for a yield node, but I guess storing >> all temps is just fine to start with - won't impact performance, >> just memory) > > I don't think we should put temps in all closure functions, but for > generators we certainly need to. That's what I meant. A "union of structs" approach would allow us to safe memory even for longer functions with many temps, as the space needed would match exactly the maximum number of temps that are in use during a yield. >> - closures have an additional C field to store the execution state >> (void* to a function label, initially NULL) > > Ah, function lablels, good idea. It might make sense to initialize it > to a label at the entry point of the function, so you can always start > with a goto *self->exec_state, but if that label is hard to get at > then setting (and checking) for NULL should work just fine. I think it'll be hard to get. Labels only exist inside of functions, as do static local variables, so you'd have to call the function in order to get to the label pointer. >> - "sendval = (yield [expr])" emits the following code: >> - store away all current temp values in the closure >> - set "closure._resume_label" to the resume label (see below, >> uses >> the C operator "&&") >> - return the expression result (or None) - return immediately >> without cleanup (the temp that holds the expression result >> must be >> unmanaged to prevent DECREF()-ing on resume; INCREF()-ing the >> return value will keep it alive for too long) >> - here goes the resume label ("__Lxyz_resume_from_yield:") >> - reset all saved temp values from the closure >> - if an exception is to be raised (gen.throw() was called, >> which has >> already set the exception externally), use normal exception >> path > > Rather than inserting this after every label, I think throw() should > be a completely separate function. It can't be because the exception must follows the normal exception flow inside the generator function. >> - set the result temp of the yield node to the send value >> argument >> that was passed (INCREF or not, as for parameters) >> * generator C function basically implements gen.send(x) >> - receives both the closure and the current send value as >> parameters >> - if "closure._resume_label" is not NULL, jump to the label; >> otherwise, check that 'x' is None (raise an exception if not) >> and >> execute the function body normally >> >> So the main work that's left to be done in 4) will be the closure >> extension >> to include the temps and the yield/resume implementation. >> >> Here's the (trivial) generic generator type: >> >> cdef class generator: >> cdef object _closure >> cdef meth_o_func* _run >> cdef object __weakref__ >> >> def __iter__(self): >> return self >> >> def __next__(self): >> return self._run(self._closure, None) >> >> def send(self, value): >> return self._run(self._closure, value) >> >> def throw(self, type, value=None, traceback=None): >> EXC_RESET(type, value, traceback) >> return self._run(self._closure, NULL) >> >> def close(self): >> try: >> EXC_RESET(GeneratorExit, NULL, NULL) >> self._run(self._closure, NULL) >> except (GeneratorExit, StopIteration): >> pass >> else: >> raise RuntimeError('generator ignored GeneratorExit') > > We also need a __dealloc__ method to clean up any stored temps, etc. > in case this generator goes out of scope before it is used up. True. In that case, we'd also have to know which temps were actually used at some point. Maybe the yield node could automatically emit corresponding cleanup code into a separate code writer that builds the __dealloc__ method. >> I wonder if there is a way to make it inherit from CPython's >> GeneratorType. >> That would enhance the interoperability, but it would also mean that >> we add >> some unnecessary instance size overhead and that we have to prevent >> that >> base-type from doing anything, including initialisation and final >> cleanup. > > That could be nice, but it could have unforeseen and nasty > consequences, especially if the generator type evolves as well. It certainly would be nasty. The CPython generator type keeps a frame alive, for example, and checks for that on type construction. So we'd either have to create a fake frame object to initialise the generator (and kill it afterwards, so that it won't get used), or find a way to disable the type initialisation completely. Rather scary either way. Stefan _______________________________________________ Cython-dev mailing list [email protected] http://codespeak.net/mailman/listinfo/cython-dev
