2010/12/8 Vitja Makarov <vitja.maka...@gmail.com>: > 2010/12/7 Vitja Makarov <vitja.maka...@gmail.com>: >> 2010/12/7 Vitja Makarov <vitja.maka...@gmail.com>: >>> 2010/12/7 Robert Bradshaw <rober...@math.washington.edu>: >>>> First off, great to hear you're working on this. I've wanted to do it >>>> for a while (well, essentially ever since I got closures done), but >>>> don't know when I'll find the time. >>>> >>>> >>>> On Mon, Dec 6, 2010 at 12:56 AM, Vitja Makarov <vitja.maka...@gmail.com> >>>> wrote: >>>> >>>>>>> def my_generator_body(closure, value, is_exception): >>>>>> >>>>>> Shouldn't that be a cdef function? >>>>> >>>>> Guess no. It has very special signature and should be used only in >>>>> generator closure. >>>>> >>>>> It can't be cdef as cdef doesn't parse (args, kwargs) and generator >>>>> does this on first run. >>>> >>>> Eventually the argument parsing code should be abstracted out so that >>>> we don't have this restriction. >>>> >>>>>>> # switch case >>>>>>> switch (closure.resume_lable): >>>>>>> case 0: >>>>>>> goto parse_arguments; >>>>>>> case 1: >>>>>>> goto resume_from_yield1; >>>>>>> parse_arguments: >>>>>>> # function body here >>>>>> >>>>>> Ok, sure, as in the CEP. >>>>>> >>>>>> >>>>>>> About temps: >>>>>>> now temps are allocated in closure using declare_var() and it works >>>>>>> fine. >>>>>> >>>>>> I think that could seriously slow down things inside of the generator, >>>>>> though. It means that it needs to jump through the closure indirection >>>>>> for >>>>>> every little value that it keeps around temporarily, including lots of >>>>>> intermediate C results. Also, I would expect that a lot more temps are >>>>>> used >>>>>> inside of the generator body than what actually needs to be kept alive >>>>>> during yields, so a single yield inside of a lengthy generator body could >>>>>> have a tremendous impact on the overall runtime and closure size. >>>>>> >>>>>> If you only store away the live temp values before a yield, you don't >>>>>> even >>>>>> need to do any reference counting for them. It's just a quick bunch of C >>>>>> assignments on yield and resume. That's about the smallest impact you >>>>>> can get. >>>>>> >>>>>> BTW, if you change the object struct of the generator closure anyway, >>>>>> maybe >>>>>> a dynamically sized PyVarObject would be a way to incorporate the temps? >>>>>> >>>>>> >>>>> >>>>> I don't understand why do you think about performance problems? >>>>> >>>>> It seems to me that if temps are stored in tuple (or another kind of >>>>> object) you have to manully save/restore them, and this code will >>>>> really slow down generator. >>>> >>>> -1 to storing them all in a tuple. That wouldn't handle C values very >>>> well, for one thing. I wouldn't use arrays though--either declare each >>>> temp individually or create a custom struct that has the correct slot >>>> for each temp. Copy to locals on enter, copy out on >>>> yield/exception/return. >>>> >>>>> On the other hand when all temps are declared in closure you just have >>>>> to say "closure->pyx_temp_v1" >>>>> >>>>> Do you think that closure-> will be much slower then stack operations? >>>> >>>> Yes, the performance difference can be dramatic. This is one of the >>>> reasons good numpy support is tricky to get right--the compiler can >>>> optimize stuff on the stack like crazy, but the same can't be said of >>>> indirect references. I'm fine with sticking all temps in the closure >>>> for now if that gets us up and running faster, and optimizing things >>>> later. >>>> >>>>>>> I'm not sure about manage_ref flag I guess that temps with >>>>>>> manage_ref=False shouldn't be allocated on closure (am I right?) >>>>>> >>>>>> "manage_ref=False" (for a Python object value) just means that the >>>>>> reference is either borrowed or will be stolen at some point. I'm not >>>>>> sure >>>>>> that stealing the reference may not happen *after* a yield, but I guess >>>>>> that would produce other problems if the reference isn't kept alive in >>>>>> another way. Nothing to worry about now, carefully crafted tests will >>>>>> eventually catch that. >>>>>> >>>>>> In any case, all live temps (Python refs, borrowed refs and C values) may >>>>>> be needed after the yield, so they must *all* end up in the closure. >>>> >>>> And of course we need to make all live reference are properly >>>> deallocated when the closure is GC'd as well. >>>> >>>> - Robert >>>> _______________________________________________ >>>> Cython-dev mailing list >>>> Cython-dev@codespeak.net >>>> http://codespeak.net/mailman/listinfo/cython-dev >>>> >>> >>> YieldExprNode can check what temps are currently used and >>> save/restore, and declare them on demand in closure. >>> Sounds not bad too. This is not a problem. >>> >>> I created now I create AbstractGeneratorClass and make it base for closure. >>> I want to bind tp_iter directly to my C functions, I found this way, >>> but it's a little bit trickey... >>> >>> entry = klass.declare_var('__iter__', PyrexTypes.py_object_type, pos, >>> visibility='extern') >>> entry.func_cname = 'PyObject_SelfIter' >>> >>> >>> I'm now stuck with generator body and function that returns genertor: >>> >>> DefNode should be translated into generator body: >>> >>> first I wanted to make it PyObject (*)(PyObject *closure, PyObject >>> *value, int is_exception) >>> But it seems to be hard now. And I'm thinking to put value and >>> is_exception into AbstractGenerator class. And let generator body >>> C-function accept original PyFunction arguments (PyObject *args, >>> PyObject *kwargs) >>> >>> Also GeneratorWrapperNode should create C-function that creates >>> closure and returns it. That's not very clean should that be attribute >>> of DefNode, or should it subclass DefNode. So I'm thinking to try it >>> in a dirty way. >>> >>> -- >>> vitja. >>> >> >> Btw it would be very nice to split DefNode into smaller parts. Not sure how. >> >> -- >> vitja. >> > > First success > > vi...@vitja-laptop:~/tmp/cython-devel-hg/zzz$ python > Python 2.6.6 (r266:84292, Sep 15 2010, 16:22:56) > [GCC 4.4.5] on linux2 > Type "help", "copyright", "credits" or "license" for more information. >>>> import generator >>>> x = generator.foo([1,2,3]) >>>> next(x) > 1 >>>> next(x) > 2 >>>> next(x) > 3 >>>> next(x) > Traceback (most recent call last): > File "<stdin>", line 1, in <module> > File "generator.pyx", line 1, in generator.foo (generator.c:518) > def foo(x): > StopIteration >>>> > > > > -- > vitja. >
Please review this patch. It's not yet finished, and mostly doesn't work. But you can take a look at patch and generated code. - Temps are saved/restored/allocated inside YieldExprNode using helper ClosureTempAllocator - PyxGenerator methods are defined via declare_var - Send, __next__(), __iter__() are implemented while close(), throw() aren't - YieldExprNode doesn't handle exceptions - I don't know how to make refnanny work, should refnanny context be stored in closure? - Objects can leak now, wouldn't check until refnanny work - GeneratorWrapperNode isn't node and is mostly a hack - Comprehensions are now handled by OldYieldExprNode - regr tests are broken -- vitja.
generators-support.patch.gz
Description: GNU Zip compressed data
_______________________________________________ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
