2010/12/7 Vitja Makarov <[email protected]>: > 2010/12/7 Robert Bradshaw <[email protected]>: >> 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 <[email protected]> >> 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 >> [email protected] >> 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. _______________________________________________ Cython-dev mailing list [email protected] http://codespeak.net/mailman/listinfo/cython-dev
