Re: [Cython] General generator expressions
2011/1/7 Stefan Behnel : > Robert Bradshaw, 07.01.2011 19:17: >> On Fri, Jan 7, 2011 at 4:27 AM, Vitja Makarov wrote: >>> I've added GenerartorDefNode and GeneratorBodyDefNode now all the >>> generators stuff is handled there. >> >> Yay! I'm pretty busy this weekend, but have been following your branch >> from a distance and so reviewing this shouldn't take too long. I'm >> thinking we should get a 0.14.1 bug fix out ASAP (e.g. once those >> Windows and type inference bugs are in) and then 0.15 will follow with >> generators. > > Absolutely. I should find some time for a review early next week, but more > eyes are always better. > I've added some new things: - return with no value inside generator - nested yields (yield (yield (yield))) - yield inside lambda (strange thing btw), was used in test_collections - genexpr support, inlining is now disabled Now I'm thinking how to enable genexpr inlining. -- vitja. ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
Robert Bradshaw, 07.01.2011 19:17: > On Fri, Jan 7, 2011 at 4:27 AM, Vitja Makarov wrote: >> I've added GenerartorDefNode and GeneratorBodyDefNode now all the >> generators stuff is handled there. > > Yay! I'm pretty busy this weekend, but have been following your branch > from a distance and so reviewing this shouldn't take too long. I'm > thinking we should get a 0.14.1 bug fix out ASAP (e.g. once those > Windows and type inference bugs are in) and then 0.15 will follow with > generators. Absolutely. I should find some time for a review early next week, but more eyes are always better. Stefan ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
On Fri, Jan 7, 2011 at 4:27 AM, Vitja Makarov wrote: > 2010/12/31 Robert Bradshaw : >> On Thu, Dec 30, 2010 at 10:05 AM, Stefan Behnel wrote: >>> Vitja Makarov, 30.12.2010 18:53: Why did you reverted decorators stuff? https://github.com/vitek/cython/commit/e0d366d9409680849e6f429992ac9724e2ad1016 >>> >>> Because I didn't get it finished, it broke the Sage build and cython-devel >>> should stay cleanly working before the release. >> >> +1 >> >>> I added a link to the build >>> failure log to the trac ticket but didn't even manage to come up with a >>> suitable test case before I had to stop working on it. >>> >>> What I implemented so far works in most cases but there are issues with >>> functions at the module scope. I didn't check the history but I think I >>> recall from the code comments that Robert worked in this area (method >>> binding) a while ago, and the comments hint at an unfinished refactoring >>> that would help with this change. >> >> I'm not working on anything in the area at the moment though (or, >> contrary to my expectations, much of anything Cython-wise... having >> too much fun hanging out with the family :). >> >> - Robert >> ___ >> Cython-dev mailing list >> Cython-dev@codespeak.net >> http://codespeak.net/mailman/listinfo/cython-dev >> > > I've added GenerartorDefNode and GeneratorBodyDefNode now all the > generators stuff is handled there. Yay! I'm pretty busy this weekend, but have been following your branch from a distance and so reviewing this shouldn't take too long. I'm thinking we should get a 0.14.1 bug fix out ASAP (e.g. once those Windows and type inference bugs are in) and then 0.15 will follow with generators. - Robert ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
2010/12/31 Robert Bradshaw : > On Thu, Dec 30, 2010 at 10:05 AM, Stefan Behnel wrote: >> Vitja Makarov, 30.12.2010 18:53: >>> Why did you reverted decorators stuff? >>> >>> https://github.com/vitek/cython/commit/e0d366d9409680849e6f429992ac9724e2ad1016 >> >> Because I didn't get it finished, it broke the Sage build and cython-devel >> should stay cleanly working before the release. > > +1 > >> I added a link to the build >> failure log to the trac ticket but didn't even manage to come up with a >> suitable test case before I had to stop working on it. >> >> What I implemented so far works in most cases but there are issues with >> functions at the module scope. I didn't check the history but I think I >> recall from the code comments that Robert worked in this area (method >> binding) a while ago, and the comments hint at an unfinished refactoring >> that would help with this change. > > I'm not working on anything in the area at the moment though (or, > contrary to my expectations, much of anything Cython-wise... having > too much fun hanging out with the family :). > > - Robert > ___ > Cython-dev mailing list > Cython-dev@codespeak.net > http://codespeak.net/mailman/listinfo/cython-dev > I've added GenerartorDefNode and GeneratorBodyDefNode now all the generators stuff is handled there. Please review. -- vitja. ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
On Thu, Dec 30, 2010 at 10:05 AM, Stefan Behnel wrote: > Vitja Makarov, 30.12.2010 18:53: >> Why did you reverted decorators stuff? >> >> https://github.com/vitek/cython/commit/e0d366d9409680849e6f429992ac9724e2ad1016 > > Because I didn't get it finished, it broke the Sage build and cython-devel > should stay cleanly working before the release. +1 > I added a link to the build > failure log to the trac ticket but didn't even manage to come up with a > suitable test case before I had to stop working on it. > > What I implemented so far works in most cases but there are issues with > functions at the module scope. I didn't check the history but I think I > recall from the code comments that Robert worked in this area (method > binding) a while ago, and the comments hint at an unfinished refactoring > that would help with this change. I'm not working on anything in the area at the moment though (or, contrary to my expectations, much of anything Cython-wise... having too much fun hanging out with the family :). - Robert ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
Vitja Makarov, 30.12.2010 18:53: > Why did you reverted decorators stuff? > > https://github.com/vitek/cython/commit/e0d366d9409680849e6f429992ac9724e2ad1016 Because I didn't get it finished, it broke the Sage build and cython-devel should stay cleanly working before the release. I added a link to the build failure log to the trac ticket but didn't even manage to come up with a suitable test case before I had to stop working on it. What I implemented so far works in most cases but there are issues with functions at the module scope. I didn't check the history but I think I recall from the code comments that Robert worked in this area (method binding) a while ago, and the comments hint at an unfinished refactoring that would help with this change. Stefan ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
2010/12/14 Vitja Makarov : > 2010/12/14 Stefan Behnel : >> Vitja Makarov, 14.12.2010 15:49: >>> 2010/12/14 Stefan Behnel: Vitja Makarov, 14.12.2010 14:45: > 2010/12/14 Stefan Behnel: >> Stefan Behnel, 13.12.2010 06:37: >>> Vitja Makarov, 12.12.2010 20:23: What do you think about GeneratorWrapperNode? >>> >>> It seems ok. I'll give it a deeper look ASAP. >> >> Actually, after looking through it, I think it's an ugly hack. It tries >> to >> hide the fact that the DefNode does the wrong thing for the two faces of >> generators, and that the current implementation does not build a separate >> node at transform time that does the right thing instead. IMHO, two nodes >> are needed. A CFuncDefNode (or a subtype) that writes out the generator C >> function with the right signature and the original body, and the original >> DefNode (with the original signature) that has its body replaced by the >> necessary generator setup code. That's basically why the original CEP >> proposed to split the DefNode implementation *before* going for >> generators. >> Would have made this easier. > > Are you sure that CFuncDefNode for generator body is a good idea? > > Nested function and classes, lambda, generators are not supported > inside CFuncDefNode. Hmm, right, that's a missing feature that could become a problem. Second try. Create a new subtype of DefNode that nails the signature to METH_O and additionally implements all the generator specific stuff, potentially through sensible hooks in DefNode's code generation. >>> >>> What is the best place to create node for generator body? >> >> "Create" as in "implement"? In Nodes.py. >> >> "Create" as in "instantiate"? Depends. The canonical answer is "in a >> transform", but sometimes it's easier to do it inside of a node. If the >> latter, the canonical answer would become analyse_types(). If that's too >> late, well, back to a transform. :) >> > > I think analyse_types() should be ok, and it wouldn't break things > Why did you reverted decorators stuff? https://github.com/vitek/cython/commit/e0d366d9409680849e6f429992ac9724e2ad1016 -- vitja. ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
2010/12/14 Stefan Behnel : > Vitja Makarov, 14.12.2010 15:49: >> 2010/12/14 Stefan Behnel: >>> Vitja Makarov, 14.12.2010 14:45: 2010/12/14 Stefan Behnel: > Stefan Behnel, 13.12.2010 06:37: >> Vitja Makarov, 12.12.2010 20:23: >>> What do you think about GeneratorWrapperNode? >> >> It seems ok. I'll give it a deeper look ASAP. > > Actually, after looking through it, I think it's an ugly hack. It tries to > hide the fact that the DefNode does the wrong thing for the two faces of > generators, and that the current implementation does not build a separate > node at transform time that does the right thing instead. IMHO, two nodes > are needed. A CFuncDefNode (or a subtype) that writes out the generator C > function with the right signature and the original body, and the original > DefNode (with the original signature) that has its body replaced by the > necessary generator setup code. That's basically why the original CEP > proposed to split the DefNode implementation *before* going for > generators. > Would have made this easier. Are you sure that CFuncDefNode for generator body is a good idea? Nested function and classes, lambda, generators are not supported inside CFuncDefNode. >>> >>> Hmm, right, that's a missing feature that could become a problem. >>> >>> Second try. Create a new subtype of DefNode that nails the signature to >>> METH_O and additionally implements all the generator specific stuff, >>> potentially through sensible hooks in DefNode's code generation. >>> >> >> What is the best place to create node for generator body? > > "Create" as in "implement"? In Nodes.py. > > "Create" as in "instantiate"? Depends. The canonical answer is "in a > transform", but sometimes it's easier to do it inside of a node. If the > latter, the canonical answer would become analyse_types(). If that's too > late, well, back to a transform. :) > I think analyse_types() should be ok, and it wouldn't break things -- vitja. ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
Vitja Makarov, 14.12.2010 15:49: > 2010/12/14 Stefan Behnel: >> Vitja Makarov, 14.12.2010 14:45: >>> 2010/12/14 Stefan Behnel: Stefan Behnel, 13.12.2010 06:37: > Vitja Makarov, 12.12.2010 20:23: >> What do you think about GeneratorWrapperNode? > > It seems ok. I'll give it a deeper look ASAP. Actually, after looking through it, I think it's an ugly hack. It tries to hide the fact that the DefNode does the wrong thing for the two faces of generators, and that the current implementation does not build a separate node at transform time that does the right thing instead. IMHO, two nodes are needed. A CFuncDefNode (or a subtype) that writes out the generator C function with the right signature and the original body, and the original DefNode (with the original signature) that has its body replaced by the necessary generator setup code. That's basically why the original CEP proposed to split the DefNode implementation *before* going for generators. Would have made this easier. >>> >>> Are you sure that CFuncDefNode for generator body is a good idea? >>> >>> Nested function and classes, lambda, generators are not supported >>> inside CFuncDefNode. >> >> Hmm, right, that's a missing feature that could become a problem. >> >> Second try. Create a new subtype of DefNode that nails the signature to >> METH_O and additionally implements all the generator specific stuff, >> potentially through sensible hooks in DefNode's code generation. >> > > What is the best place to create node for generator body? "Create" as in "implement"? In Nodes.py. "Create" as in "instantiate"? Depends. The canonical answer is "in a transform", but sometimes it's easier to do it inside of a node. If the latter, the canonical answer would become analyse_types(). If that's too late, well, back to a transform. :) Stefan ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
2010/12/14 Stefan Behnel : > Vitja Makarov, 14.12.2010 14:45: >> 2010/12/14 Stefan Behnel: >>> Stefan Behnel, 13.12.2010 06:37: Vitja Makarov, 12.12.2010 20:23: > What do you think about GeneratorWrapperNode? It seems ok. I'll give it a deeper look ASAP. >>> >>> Actually, after looking through it, I think it's an ugly hack. It tries to >>> hide the fact that the DefNode does the wrong thing for the two faces of >>> generators, and that the current implementation does not build a separate >>> node at transform time that does the right thing instead. IMHO, two nodes >>> are needed. A CFuncDefNode (or a subtype) that writes out the generator C >>> function with the right signature and the original body, and the original >>> DefNode (with the original signature) that has its body replaced by the >>> necessary generator setup code. That's basically why the original CEP >>> proposed to split the DefNode implementation *before* going for generators. >>> Would have made this easier. >> >> Are you sure that CFuncDefNode for generator body is a good idea? >> >> Nested function and classes, lambda, generators are not supported >> inside CFuncDefNode. > > Hmm, right, that's a missing feature that could become a problem. > > Second try. Create a new subtype of DefNode that nails the signature to > METH_O and additionally implements all the generator specific stuff, > potentially through sensible hooks in DefNode's code generation. > What is the best place to create node for generator body? In analyse_...? > >> Btw. It would be nice to make DefNode base for GeneratorNode, all the >> generator creation stuff could be moved into generate_function_body() >> and make generator body childern of GeneratorNode. > > I'm having trouble parsing the last part of this sentence, but it seems we > are agreeing that a subtype would be a good idea. > oops -- vitja. ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
Vitja Makarov, 14.12.2010 14:45: > 2010/12/14 Stefan Behnel: >> Stefan Behnel, 13.12.2010 06:37: >>> Vitja Makarov, 12.12.2010 20:23: What do you think about GeneratorWrapperNode? >>> >>> It seems ok. I'll give it a deeper look ASAP. >> >> Actually, after looking through it, I think it's an ugly hack. It tries to >> hide the fact that the DefNode does the wrong thing for the two faces of >> generators, and that the current implementation does not build a separate >> node at transform time that does the right thing instead. IMHO, two nodes >> are needed. A CFuncDefNode (or a subtype) that writes out the generator C >> function with the right signature and the original body, and the original >> DefNode (with the original signature) that has its body replaced by the >> necessary generator setup code. That's basically why the original CEP >> proposed to split the DefNode implementation *before* going for generators. >> Would have made this easier. > > Are you sure that CFuncDefNode for generator body is a good idea? > > Nested function and classes, lambda, generators are not supported > inside CFuncDefNode. Hmm, right, that's a missing feature that could become a problem. Second try. Create a new subtype of DefNode that nails the signature to METH_O and additionally implements all the generator specific stuff, potentially through sensible hooks in DefNode's code generation. > Btw. It would be nice to make DefNode base for GeneratorNode, all the > generator creation stuff could be moved into generate_function_body() > and make generator body childern of GeneratorNode. I'm having trouble parsing the last part of this sentence, but it seems we are agreeing that a subtype would be a good idea. > I'm thinking of genexprs they are very similar to lambdas and could be > implemented reusing lambda stuff. Lambda is basically implemented as a little transform that creates a DefNode. I agree, it should be enough to just wrap generator expressions in a new DefNode and let the normal generator machinery do its work. However, note that this needs to interact with the EarlyReplaceBuiltinCalls transform in some way, which inlines generator expressions if possible. Looks like it's going to be tricky to arrange the related transformations according to the interdependencies, e.g. to the AnalyseDeclarationsTransform. I guess subtyping DefNode in two levels (DefNode<-GeneratorNode<-GeneratorExpressionNode) could be a step to fix this, as the inlining transform could then be changed to properly unpack that node instead of the current plain expression. Stefan ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
2010/12/14 Stefan Behnel : > Stefan Behnel, 13.12.2010 06:37: >> Vitja Makarov, 12.12.2010 20:23: >>> What do you think about GeneratorWrapperNode? >> >> It seems ok. I'll give it a deeper look ASAP. > > Actually, after looking through it, I think it's an ugly hack. It tries to > hide the fact that the DefNode does the wrong thing for the two faces of > generators, and that the current implementation does not build a separate > node at transform time that does the right thing instead. IMHO, two nodes > are needed. A CFuncDefNode (or a subtype) that writes out the generator C > function with the right signature and the original body, and the original > DefNode (with the original signature) that has its body replaced by the > necessary generator setup code. That's basically why the original CEP > proposed to split the DefNode implementation *before* going for generators. > Would have made this easier. > Are you sure that CFuncDefNode for generator body is a good idea? Nested function and classes, lambda, generators are not supported inside CFuncDefNode. Btw. It would be nice to make DefNode base for GeneratorNode, all the generator creation stuff could be moved into generate_function_body() and make generator body childern of GeneratorNode. I'm thinking of genexprs they are very similar to lambdas and could be implemented reusing lambda stuff. -- vitja. ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
Stefan Behnel, 13.12.2010 06:37: > Vitja Makarov, 12.12.2010 20:23: >> What do you think about GeneratorWrapperNode? > > It seems ok. I'll give it a deeper look ASAP. Actually, after looking through it, I think it's an ugly hack. It tries to hide the fact that the DefNode does the wrong thing for the two faces of generators, and that the current implementation does not build a separate node at transform time that does the right thing instead. IMHO, two nodes are needed. A CFuncDefNode (or a subtype) that writes out the generator C function with the right signature and the original body, and the original DefNode (with the original signature) that has its body replaced by the necessary generator setup code. That's basically why the original CEP proposed to split the DefNode implementation *before* going for generators. Would have made this easier. Stefan ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
2010/12/13 Stefan Behnel : > Vitja Makarov, 13.12.2010 13:54: >> 2010/12/13 Stefan Behnel: >>> Vitja Makarov, 12.12.2010 20:23: It seems that there is no major issues with generators, refleaks and so on. >>> >>> I know what the refcounting problem is now. It's a bug that has already >>> been (mostly) fixed in Haoyu's branch. So far, we didn't have "nonlocal", >>> so there was no way to test how closure variables behaved on assignment. >>> Since local variables in generator functions now live in the closure and >>> can happily get assigned to, they suffer from the same problem that >>> nonlocal has. >>> >>> The place to fix this is in the assignment code in NameNode. For values in >>> closures, it must generate a GOTREF() before DECREF-ing the lhs in the >>> assignment and a GIVREF() after the assignment. I fixed that and also >>> ripped out the "make refnanny happy" code sections. >>> >>> Doing that, I also noticed a couple of other related refnanny problems for >>> argument handling. I didn't care to port them back to mainline since I'd >>> expect that the current 0.14 release can live with them. Let's assume from >>> the current state of the generators branch that 0.15 is not too far off. >> >> Did you already fixed this bug? > > Yes, and *now* it's even up on github. :) > > Stefan Cool :)) -- vitja. ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
Vitja Makarov, 13.12.2010 13:54: > 2010/12/13 Stefan Behnel: >> Vitja Makarov, 12.12.2010 20:23: >>> It seems that there is no major issues with generators, refleaks and so on. >> >> I know what the refcounting problem is now. It's a bug that has already >> been (mostly) fixed in Haoyu's branch. So far, we didn't have "nonlocal", >> so there was no way to test how closure variables behaved on assignment. >> Since local variables in generator functions now live in the closure and >> can happily get assigned to, they suffer from the same problem that >> nonlocal has. >> >> The place to fix this is in the assignment code in NameNode. For values in >> closures, it must generate a GOTREF() before DECREF-ing the lhs in the >> assignment and a GIVREF() after the assignment. I fixed that and also >> ripped out the "make refnanny happy" code sections. >> >> Doing that, I also noticed a couple of other related refnanny problems for >> argument handling. I didn't care to port them back to mainline since I'd >> expect that the current 0.14 release can live with them. Let's assume from >> the current state of the generators branch that 0.15 is not too far off. > > Did you already fixed this bug? Yes, and *now* it's even up on github. :) Stefan ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
2010/12/13 Stefan Behnel : > Vitja Makarov, 13.12.2010 07:49: >> 2010/12/13 Stefan Behnel: >>> Vitja Makarov, 12.12.2010 20:23: I've cleaned generators stuff a little: * Turn YieldNodeCollector into TreeVisitor >>> >>> I think that's ok. It's a bit redundant with the tree traversal that >>> happens anyway, but it also keeps the code more compact and makes it more >>> understandable. >>> >>> BTW, please start using error test cases. That would easily have caught the >>> obvious reporting bug in visit_ReturnStatNode. >> >> I was thinking about that. I also found that 'return' statment is >> allowed inside generators. >> That should raise StopIteration and close generator. >> Don't know how to go here replace ReturnStatNode with another one > > You can replace it by the equivalent of "raise StopIteration" during the > MarkClosureVisitor transform, e.g. inside of the YieldNodeCollector (which > would then have to be a transform again). Either emit a compiler error if > the return statement has an argument (IIRC that's the forbidden case) or > replace the node if it has none. > > That shold be much better to jump to the place where StopIteration is raised. Couse StopIteration should not be catched here and finallize should be called. * Move ClosureTempAllocator to Code.py >>> >>> I still think that the reference should live in FunctionState so that it >>> goes out of scope automatically. It shouldn't live longer than the function. >>> >> >> Ok, I'll put reference to code.funcstate, or maybe add method to >> funcstate set_closure_class() >> That will create tempallocator? > > Hmm, not sure. What about adding an "init_generator_temps(scope)" method to > it that creates the CTA and attaches it to the current funcstate? > Ok > Stefan > ___ > Cython-dev mailing list > Cython-dev@codespeak.net > http://codespeak.net/mailman/listinfo/cython-dev > -- vitja. ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
2010/12/13 Stefan Behnel : > Vitja Makarov, 12.12.2010 20:23: >> It seems that there is no major issues with generators, refleaks and so on. > > I know what the refcounting problem is now. It's a bug that has already > been (mostly) fixed in Haoyu's branch. So far, we didn't have "nonlocal", > so there was no way to test how closure variables behaved on assignment. > Since local variables in generator functions now live in the closure and > can happily get assigned to, they suffer from the same problem that > nonlocal has. > > The place to fix this is in the assignment code in NameNode. For values in > closures, it must generate a GOTREF() before DECREF-ing the lhs in the > assignment and a GIVREF() after the assignment. I fixed that and also > ripped out the "make refnanny happy" code sections. > > Doing that, I also noticed a couple of other related refnanny problems for > argument handling. I didn't care to port them back to mainline since I'd > expect that the current 0.14 release can live with them. Let's assume from > the current state of the generators branch that 0.15 is not too far off. > > Stefan Did you already fixed this bug? -- vitja. ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
Vitja Makarov, 13.12.2010 07:49: > 2010/12/13 Stefan Behnel: >> Vitja Makarov, 12.12.2010 20:23: >>> I've cleaned generators stuff a little: >>> >>>* Turn YieldNodeCollector into TreeVisitor >> >> I think that's ok. It's a bit redundant with the tree traversal that >> happens anyway, but it also keeps the code more compact and makes it more >> understandable. >> >> BTW, please start using error test cases. That would easily have caught the >> obvious reporting bug in visit_ReturnStatNode. > > I was thinking about that. I also found that 'return' statment is > allowed inside generators. > That should raise StopIteration and close generator. > Don't know how to go here replace ReturnStatNode with another one You can replace it by the equivalent of "raise StopIteration" during the MarkClosureVisitor transform, e.g. inside of the YieldNodeCollector (which would then have to be a transform again). Either emit a compiler error if the return statement has an argument (IIRC that's the forbidden case) or replace the node if it has none. >>>* Move ClosureTempAllocator to Code.py >> >> I still think that the reference should live in FunctionState so that it >> goes out of scope automatically. It shouldn't live longer than the function. >> > > Ok, I'll put reference to code.funcstate, or maybe add method to > funcstate set_closure_class() > That will create tempallocator? Hmm, not sure. What about adding an "init_generator_temps(scope)" method to it that creates the CTA and attaches it to the current funcstate? Stefan ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
Vitja Makarov, 12.12.2010 20:23: > It seems that there is no major issues with generators, refleaks and so on. I know what the refcounting problem is now. It's a bug that has already been (mostly) fixed in Haoyu's branch. So far, we didn't have "nonlocal", so there was no way to test how closure variables behaved on assignment. Since local variables in generator functions now live in the closure and can happily get assigned to, they suffer from the same problem that nonlocal has. The place to fix this is in the assignment code in NameNode. For values in closures, it must generate a GOTREF() before DECREF-ing the lhs in the assignment and a GIVREF() after the assignment. I fixed that and also ripped out the "make refnanny happy" code sections. Doing that, I also noticed a couple of other related refnanny problems for argument handling. I didn't care to port them back to mainline since I'd expect that the current 0.14 release can live with them. Let's assume from the current state of the generators branch that 0.15 is not too far off. Stefan ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
2010/12/13 Stefan Behnel : > Vitja Makarov, 12.12.2010 20:23: >> I've cleaned generators stuff a little: >> >> * Turn YieldNodeCollector into TreeVisitor > > I think that's ok. It's a bit redundant with the tree traversal that > happens anyway, but it also keeps the code more compact and makes it more > understandable. > > BTW, please start using error test cases. That would easily have caught the > obvious reporting bug in visit_ReturnStatNode. > > I was thinking about that. I also found that 'return' statment is allowed inside generators. That should raise StopIteration and close generator. Don't know how to go here replace ReturnStatNode with another one or set attribute of return node: node.return_from_generator = True? >> * Merge MarkClosureVisitor and MarkGeneratorVisitor > > That looks much better. > > >> * Rename stuff from CyGenerator to __Pyx_Generator > > Thanks. However, the following doesn't seem to have an effect, does it? > > - entry.cname = 'CyGenerator' > + entry.cname = 'Generator' > > If that's the only place where that cname is used, I'd guess that it should > better be dropped. > Sure. Obsolete code ) > >> * Move ClosureTempAllocator to Code.py > > I still think that the reference should live in FunctionState so that it > goes out of scope automatically. It shouldn't live longer than the function. > Ok, I'll put reference to code.funcstate, or maybe add method to funcstate set_closure_class() That will create tempallocator? > >> It seems that there is no major issues with generators, refleaks and so on. > > I'm not so sure about the ref-counting yet. I'll have to think it through a > bit more, but it currently feels a bit funny. (That doesn't mean it's > wrong, just that I'm not sure it's the way it should be) > > >> So now is time to add support for genexps and code cleanup. > > Absolutely. :) > > >> What do you think about GeneratorWrapperNode? > > It seems ok. I'll give it a deeper look ASAP. > ok. -- vitja. ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
Vitja Makarov, 12.12.2010 20:23: > I've cleaned generators stuff a little: > > * Turn YieldNodeCollector into TreeVisitor I think that's ok. It's a bit redundant with the tree traversal that happens anyway, but it also keeps the code more compact and makes it more understandable. BTW, please start using error test cases. That would easily have caught the obvious reporting bug in visit_ReturnStatNode. > * Merge MarkClosureVisitor and MarkGeneratorVisitor That looks much better. > * Rename stuff from CyGenerator to __Pyx_Generator Thanks. However, the following doesn't seem to have an effect, does it? -entry.cname = 'CyGenerator' +entry.cname = 'Generator' If that's the only place where that cname is used, I'd guess that it should better be dropped. > * Move ClosureTempAllocator to Code.py I still think that the reference should live in FunctionState so that it goes out of scope automatically. It shouldn't live longer than the function. > It seems that there is no major issues with generators, refleaks and so on. I'm not so sure about the ref-counting yet. I'll have to think it through a bit more, but it currently feels a bit funny. (That doesn't mean it's wrong, just that I'm not sure it's the way it should be) > So now is time to add support for genexps and code cleanup. Absolutely. :) > What do you think about GeneratorWrapperNode? It seems ok. I'll give it a deeper look ASAP. Stefan ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
2010/12/11 Vitja Makarov : > 2010/12/11 Stefan Behnel : >> Vitja Makarov, 10.12.2010 09:22: >>> 2010/12/10 Stefan Behnel: Stefan Behnel, 09.12.2010 10:46: > Vitja Makarov, 08.12.2010 22:45: >> 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. > > I'll take a close look ASAP. > >> - Temps are saved/restored/allocated inside YieldExprNode using >> helper ClosureTempAllocator Ok, that currently lives in ParseTreeTransforms, which is totally the wrong place for anything that generates code. It also mixes functionality that normally lives in Symtab.py with functionality that lives in Code.py. >>> >>> Yes, I know. That was placed there to give it a try (actually I'm not >>> sure what is the best place for it). ClosureTempAllocator now is >>> attribute of defnode and all yieldnodes, that's not clean. >> >> FunctionState in Code.py seems like a good place to hold a reference, at >> least for the code generation part of it. >> > > At some point we can call code.set_closure_class() > that would create code.temp_allocator and then refer to it as > code.temp_allocator. > temp_allocator isn't good name here maybe closure_temps? > >> >>> ExprNode.StopIterationNode isn't expression should it be moved to Nodes.py? >> >> Why not drop it completely and generate the code in the function node >> instead? ISTM that the only reason it's there is to drop one line of code >> at the end of the function body. That can be done much easier. >> >> > > Sure. You've already done this, thanks! > >>> create_abstract_generator() is rather dirty >> >> I've seen worse. ;) >> >> >>> I don't know how to bind C functions to CClass. >> >> Such as? >> >> > > e = declare() > e.func_cname = ... > e.signature = ... > I think that there should be a short way > >>> I think that generator body could have PyObject *body(PyObject *self, >>> PyObject *value) signature >>> With value=Null meaning exception. >> >> See the CEP. ;) >> > > Yeah! Forget about that, that's already done. I was looking into > python implementation and it has `i_exc` arg. > >> >> - PyxGenerator methods are defined via declare_var > > Should we call that "CyGenerator" ? :) I actually meant to call it "CyGenerator" at the Python level, but I now see that it's actually a purely internal thing. That makes __Pyx_Generator a better choice, given that "__pyx_" is the globally reserved prefix in Cython code. >>> >>> Should rename it back ;) >> >> Yep. >> > > __pyx_CyGenerator_(Send|Close)() <-- or lover case suffix is better, > should look at pep7? > class __pyx_CyGenerator > type __pyx_CyGeneratorType > > is that ok? > >> >>> How can I declare function entry with METH_VARGS signature? >>> Using TypeSlots I can add METH_VARGS|METH_KWARGS entries but not >>> single METH_VARGS >>> >>> That's preferd for "throw" method. >> >> I see what you mean. It's a minor issue, though. Just ignore it for now, or >> raise an exception if kwargs are passed in. After all, .throw() is rarely >> used and not performance critical at all. >> > > ok > >>> Pyregr tests works now on hudson but there is still problem with heapq >>> don't understand is that related to generators or not >> >> Not sure either. It works for me in Py3 but not in Py2.7. > > I see you've fixed this issue. Cool. > >> disable generators test in Py2.4 - depends on GeneratorExit and potentially >> other stuff > > May be it's better to generate another exception (StopIteration) in > 2.4 and control this with #ifdefs ... #endif > > What do you think about generate_preamble? Think it's not correct name. > > In GeneratorWrapper() buffer cleanup code isn't implemented when > closure is created. > > What is the place for YieldCollector? > > About genexp: now it should be easy to implement just like lambda function > > -- > vitja. > I've cleaned generators stuff a little: * Turn YieldNodeCollector into TreeVisitor * Merge MarkClosureVisitor and MarkGeneratorVisitor * Rename stuff from CyGenerator to __Pyx_Generator * Move ClosureTempAllocator to Code.py It seems that there is no major issues with generators, refleaks and so on. So now is time to add support for genexps and code cleanup. What do you think about GeneratorWrapperNode? -- vitja. ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
2010/12/11 Stefan Behnel : > Vitja Makarov, 10.12.2010 09:22: >> 2010/12/10 Stefan Behnel: >>> Stefan Behnel, 09.12.2010 10:46: Vitja Makarov, 08.12.2010 22:45: > 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. I'll take a close look ASAP. > - Temps are saved/restored/allocated inside YieldExprNode using > helper ClosureTempAllocator >>> >>> Ok, that currently lives in ParseTreeTransforms, which is totally the wrong >>> place for anything that generates code. It also mixes functionality that >>> normally lives in Symtab.py with functionality that lives in Code.py. >>> >> >> Yes, I know. That was placed there to give it a try (actually I'm not >> sure what is the best place for it). ClosureTempAllocator now is >> attribute of defnode and all yieldnodes, that's not clean. > > FunctionState in Code.py seems like a good place to hold a reference, at > least for the code generation part of it. > At some point we can call code.set_closure_class() that would create code.temp_allocator and then refer to it as code.temp_allocator. temp_allocator isn't good name here maybe closure_temps? > >> ExprNode.StopIterationNode isn't expression should it be moved to Nodes.py? > > Why not drop it completely and generate the code in the function node > instead? ISTM that the only reason it's there is to drop one line of code > at the end of the function body. That can be done much easier. > > Sure. You've already done this, thanks! >> create_abstract_generator() is rather dirty > > I've seen worse. ;) > > >> I don't know how to bind C functions to CClass. > > Such as? > > e = declare() e.func_cname = ... e.signature = ... I think that there should be a short way >> I think that generator body could have PyObject *body(PyObject *self, >> PyObject *value) signature >> With value=Null meaning exception. > > See the CEP. ;) > Yeah! Forget about that, that's already done. I was looking into python implementation and it has `i_exc` arg. > > - PyxGenerator methods are defined via declare_var Should we call that "CyGenerator" ? :) >>> >>> I actually meant to call it "CyGenerator" at the Python level, but I now >>> see that it's actually a purely internal thing. That makes __Pyx_Generator >>> a better choice, given that "__pyx_" is the globally reserved prefix in >>> Cython code. >> >> Should rename it back ;) > > Yep. > __pyx_CyGenerator_(Send|Close)() <-- or lover case suffix is better, should look at pep7? class __pyx_CyGenerator type __pyx_CyGeneratorType is that ok? > >> How can I declare function entry with METH_VARGS signature? >> Using TypeSlots I can add METH_VARGS|METH_KWARGS entries but not >> single METH_VARGS >> >> That's preferd for "throw" method. > > I see what you mean. It's a minor issue, though. Just ignore it for now, or > raise an exception if kwargs are passed in. After all, .throw() is rarely > used and not performance critical at all. > ok >> Pyregr tests works now on hudson but there is still problem with heapq >> don't understand is that related to generators or not > > Not sure either. It works for me in Py3 but not in Py2.7. I see you've fixed this issue. Cool. > disable generators test in Py2.4 - depends on GeneratorExit and potentially > other stuff May be it's better to generate another exception (StopIteration) in 2.4 and control this with #ifdefs ... #endif What do you think about generate_preamble? Think it's not correct name. In GeneratorWrapper() buffer cleanup code isn't implemented when closure is created. What is the place for YieldCollector? About genexp: now it should be easy to implement just like lambda function -- vitja. ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
Stefan Behnel, 11.12.2010 00:46: > Vitja Makarov, 10.12.2010 23:42: >> Pyregr tests works now on hudson but there is still problem with heapq >> don't understand is that related to generators or not > > Not sure either. It works for me in Py3 but not in Py2.7. It seems to be generator related and it looks like there's a bug in the exception handling. It crashes when checking for exceptions after running through the heapiter() generator method. These things often point at a dangling reference in Python's exception state. Stefan ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
Vitja Makarov, 10.12.2010 23:42: > Pyregr tests works now on hudson but there is still problem with heapq > don't understand is that related to generators or not Not sure either. It works for me in Py3 but not in Py2.7. Stefan ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
Vitja Makarov, 10.12.2010 09:22: > 2010/12/10 Stefan Behnel: >> Stefan Behnel, 09.12.2010 10:46: >>> Vitja Makarov, 08.12.2010 22:45: 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. >>> >>> I'll take a close look ASAP. >>> - Temps are saved/restored/allocated inside YieldExprNode using helper ClosureTempAllocator >> >> Ok, that currently lives in ParseTreeTransforms, which is totally the wrong >> place for anything that generates code. It also mixes functionality that >> normally lives in Symtab.py with functionality that lives in Code.py. >> > > Yes, I know. That was placed there to give it a try (actually I'm not > sure what is the best place for it). ClosureTempAllocator now is > attribute of defnode and all yieldnodes, that's not clean. FunctionState in Code.py seems like a good place to hold a reference, at least for the code generation part of it. > ExprNode.StopIterationNode isn't expression should it be moved to Nodes.py? Why not drop it completely and generate the code in the function node instead? ISTM that the only reason it's there is to drop one line of code at the end of the function body. That can be done much easier. > create_abstract_generator() is rather dirty I've seen worse. ;) > I don't know how to bind C functions to CClass. Such as? > I think that generator body could have PyObject *body(PyObject *self, > PyObject *value) signature > With value=Null meaning exception. See the CEP. ;) - PyxGenerator methods are defined via declare_var >>> >>> Should we call that "CyGenerator" ? :) >> >> I actually meant to call it "CyGenerator" at the Python level, but I now >> see that it's actually a purely internal thing. That makes __Pyx_Generator >> a better choice, given that "__pyx_" is the globally reserved prefix in >> Cython code. > > Should rename it back ;) Yep. > How can I declare function entry with METH_VARGS signature? > Using TypeSlots I can add METH_VARGS|METH_KWARGS entries but not > single METH_VARGS > > That's preferd for "throw" method. I see what you mean. It's a minor issue, though. Just ignore it for now, or raise an exception if kwargs are passed in. After all, .throw() is rarely used and not performance critical at all. Stefan ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
On Fri, Dec 10, 2010 at 2:43 PM, Vitja Makarov wrote: > 2010/12/11 Robert Bradshaw : >> On Fri, Dec 10, 2010 at 2:38 PM, Stefan Behnel wrote: >>> Vitja Makarov, 10.12.2010 22:01: I've pushed commit that moves exc_save_vars into temps >>> >>> Yes, I think that makes good sense and should simplify things a lot. >>> (now it's declared with type=py_object_type and manage_refs=False) >>> >>> Sounds right. >> >> Can't their be multiple nestings of exc_save_vars (if try statements >> are nested)? >> >> - Robert > > That should be ok, temps are allocated dynamicly. Take a look at commit > please. +1, that should be fine. (I'd actually been meaning to take a good look at your code when I had some time and then respond with any comments, including this concern were it was relevant, but just wanted to confirm this was considered when I saw someone else answered.) - Robert ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
2010/12/11 Robert Bradshaw : > On Fri, Dec 10, 2010 at 2:38 PM, Stefan Behnel wrote: >> Vitja Makarov, 10.12.2010 22:01: >>> I've pushed commit that moves exc_save_vars into temps >> >> Yes, I think that makes good sense and should simplify things a lot. >> >>> (now it's declared with type=py_object_type and manage_refs=False) >> >> Sounds right. > > Can't their be multiple nestings of exc_save_vars (if try statements > are nested)? > > - Robert That should be ok, temps are allocated dynamicly. Take a look at commit please. -- vitja. ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
On Fri, Dec 10, 2010 at 2:38 PM, Stefan Behnel wrote: > Vitja Makarov, 10.12.2010 22:01: >> I've pushed commit that moves exc_save_vars into temps > > Yes, I think that makes good sense and should simplify things a lot. > >> (now it's declared with type=py_object_type and manage_refs=False) > > Sounds right. Can't their be multiple nestings of exc_save_vars (if try statements are nested)? - Robert ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
2010/12/11 Stefan Behnel : > Vitja Makarov, 10.12.2010 22:01: >> I've pushed commit that moves exc_save_vars into temps > > Yes, I think that makes good sense and should simplify things a lot. > Here is commit for it https://github.com/vitek/cython/commit/403fe34aa5ca0a9e512755b7d403cd9965a8d830 > >> (now it's declared with type=py_object_type and manage_refs=False) > > Sounds right. > > Stefan Pyregr tests works now on hudson but there is still problem with heapq don't understand is that related to generators or not -- vitja. ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
Vitja Makarov, 10.12.2010 22:01: > I've pushed commit that moves exc_save_vars into temps Yes, I think that makes good sense and should simplify things a lot. > (now it's declared with type=py_object_type and manage_refs=False) Sounds right. Stefan ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
2010/12/10 Robert Bradshaw : > On Fri, Dec 10, 2010 at 11:51 AM, Stefan Behnel wrote: >> Robert Bradshaw, 10.12.2010 20:35: >>> On Fri, Dec 10, 2010 at 11:15 AM, Vitja Makarov wrote: 2010/12/10 Robert Bradshaw: > On Fri, Dec 10, 2010 at 12:22 AM, Vitja Makarov wrote: >> 2010/12/10 Stefan Behnel: >>> I could also add Hudson jobs for it. Robert, would it be ok to add them >>> for >>> all Python versions? > > I think that would be excessive. Lets just do 2.4, >>> >>> (I meant 2.3 here...) >> >> The 2.3 tests do not run the doctests. I have no idea why (likely a bug or >> missing feature in the old doctest or unittest module), and I didn't care >> much since 2.3 is long dead anyway. In any case, 2.4 is a better choice. > > Ah, OK, I thought it was an oversight and just went through trying to > change it (which you probably noticed). Sure, 2.4 is fine. > > - Robert > ___ > Cython-dev mailing list > Cython-dev@codespeak.net > http://codespeak.net/mailman/listinfo/cython-dev > I've pushed commit that moves exc_save_vars into temps (now it's declared with type=py_object_type and manage_refs=False) Is that ok? That's required for propper generator support to store exc_save_vars somewhere. -- vitja. ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
On Fri, Dec 10, 2010 at 11:51 AM, Stefan Behnel wrote: > Robert Bradshaw, 10.12.2010 20:35: >> On Fri, Dec 10, 2010 at 11:15 AM, Vitja Makarov wrote: >>> 2010/12/10 Robert Bradshaw: On Fri, Dec 10, 2010 at 12:22 AM, Vitja Makarov wrote: > 2010/12/10 Stefan Behnel: >> I could also add Hudson jobs for it. Robert, would it be ok to add them >> for >> all Python versions? I think that would be excessive. Lets just do 2.4, >> >> (I meant 2.3 here...) > > The 2.3 tests do not run the doctests. I have no idea why (likely a bug or > missing feature in the old doctest or unittest module), and I didn't care > much since 2.3 is long dead anyway. In any case, 2.4 is a better choice. Ah, OK, I thought it was an oversight and just went through trying to change it (which you probably noticed). Sure, 2.4 is fine. - Robert ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
Robert Bradshaw, 10.12.2010 20:35: > On Fri, Dec 10, 2010 at 11:15 AM, Vitja Makarov wrote: >> 2010/12/10 Robert Bradshaw: >>> On Fri, Dec 10, 2010 at 12:22 AM, Vitja Makarov wrote: 2010/12/10 Stefan Behnel: > I could also add Hudson jobs for it. Robert, would it be ok to add them > for > all Python versions? >>> >>> I think that would be excessive. Lets just do 2.4, > > (I meant 2.3 here...) The 2.3 tests do not run the doctests. I have no idea why (likely a bug or missing feature in the old doctest or unittest module), and I didn't care much since 2.3 is long dead anyway. In any case, 2.4 is a better choice. >>> 2.7, and a 3.x. If >>> there's a strange corner case that only manifests in, say, 2.5 then >>> we'll deal with it later. >>> >> >> Can you please add pyregr for 2.7 also? > > Sure, that makes sense as well. Done. Stefan ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
On Fri, Dec 10, 2010 at 11:15 AM, Vitja Makarov wrote:
> 2010/12/10 Robert Bradshaw :
>> On Fri, Dec 10, 2010 at 12:22 AM, Vitja Makarov
>> wrote:
>>> 2010/12/10 Stefan Behnel :
Stefan Behnel, 09.12.2010 10:46:
> Vitja Makarov, 08.12.2010 22:45:
>> 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.
>
> I'll take a close look ASAP.
>
>> - Temps are saved/restored/allocated inside YieldExprNode using
>> helper ClosureTempAllocator
Ok, that currently lives in ParseTreeTransforms, which is totally the wrong
place for anything that generates code. It also mixes functionality that
normally lives in Symtab.py with functionality that lives in Code.py.
>>>
>>> Yes, I know. That was placed there to give it a try (actually I'm not
>>> sure what is the best place for it). ClosureTempAllocator now is
>>> attribute of defnode and all yieldnodes, that's not clean.
>>>
>>> ExprNode.StopIterationNode isn't expression should it be moved to Nodes.py?
>>>
>>> create_abstract_generator() is rather dirty
>>>
>>> I don't know how to bind C functions to CClass.
>>>
>>> I think that generator body could have PyObject *body(PyObject *self,
>>> PyObject *value) signature
>>> With value=Null meaning exception.
>>>
Could you give me ("scoder", or better all Cython devs) commit rights to
your repo so that I can start refactoring the code and fix problems I find?
>>>
>>> I've added you, but I don't know how to add all devs together.
>>
>> I don't think there's a way to do that, just add whoever you want help
>> participating on this branch. My username is robertwb, and though I
>> haven't rally had time to read any code yet, I'm very interested and
>> would like to help out if I can.
>>
>
> I've added you too.
Thanks.
I could also add Hudson jobs for it. Robert, would it be ok to add them for
all Python versions?
>>
>> I think that would be excessive. Lets just do 2.4,
(I meant 2.3 here...)
>> 2.7, and a 3.x. If
>> there's a strange corner case that only manifests in, say, 2.5 then
>> we'll deal with it later.
>>
>
> Can you please add pyregr for 2.7 also?
Sure, that makes sense as well.
- Robert
___
Cython-dev mailing list
Cython-dev@codespeak.net
http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
2010/12/10 Robert Bradshaw :
> On Fri, Dec 10, 2010 at 12:22 AM, Vitja Makarov
> wrote:
>> 2010/12/10 Stefan Behnel :
>>> Stefan Behnel, 09.12.2010 10:46:
Vitja Makarov, 08.12.2010 22:45:
> 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.
I'll take a close look ASAP.
> - Temps are saved/restored/allocated inside YieldExprNode using
> helper ClosureTempAllocator
>>>
>>> Ok, that currently lives in ParseTreeTransforms, which is totally the wrong
>>> place for anything that generates code. It also mixes functionality that
>>> normally lives in Symtab.py with functionality that lives in Code.py.
>>>
>>
>> Yes, I know. That was placed there to give it a try (actually I'm not
>> sure what is the best place for it). ClosureTempAllocator now is
>> attribute of defnode and all yieldnodes, that's not clean.
>>
>> ExprNode.StopIterationNode isn't expression should it be moved to Nodes.py?
>>
>> create_abstract_generator() is rather dirty
>>
>> I don't know how to bind C functions to CClass.
>>
>> I think that generator body could have PyObject *body(PyObject *self,
>> PyObject *value) signature
>> With value=Null meaning exception.
>>
>>> Could you give me ("scoder", or better all Cython devs) commit rights to
>>> your repo so that I can start refactoring the code and fix problems I find?
>>>
>>
>> I've added you, but I don't know how to add all devs together.
>
> I don't think there's a way to do that, just add whoever you want help
> participating on this branch. My username is robertwb, and though I
> haven't rally had time to read any code yet, I'm very interested and
> would like to help out if I can.
>
I've added you too.
>>> I could also add Hudson jobs for it. Robert, would it be ok to add them for
>>> all Python versions?
>
> I think that would be excessive. Lets just do 2.4, 2.7, and a 3.x. If
> there's a strange corner case that only manifests in, say, 2.5 then
> we'll deal with it later.
>
Can you please add pyregr for 2.7 also?
>> Yeah, please that would be nice.
>>
> - PyxGenerator methods are defined via declare_var
Should we call that "CyGenerator" ? :)
>>>
>>> I actually meant to call it "CyGenerator" at the Python level, but I now
>>> see that it's actually a purely internal thing. That makes __Pyx_Generator
>>> a better choice, given that "__pyx_" is the globally reserved prefix in
>>> Cython code.
>>>
>>
>> Should rename it back ;)
>>
>>> Eventually, we may(!) want to reuse the superclass across modules, but I
>>> currently have no idea how that could be made to work. So it's just another
>>> thing that we don't need to worry about now.
>>>
>>
>> Okay.
>>
>> How can I declare function entry with METH_VARGS signature?
>> Using TypeSlots I can add METH_VARGS|METH_KWARGS entries but not
>> single METH_VARGS
>>
>> That's preferd for "throw" method.
>
> Just let it be METH_VARGS|METH_KWARGS for now. That doesn't mean it
> can actually take arbitrary keyword arguments, just that the arguments
> that are needed can be passed in as keywords.
>
Ok
--
vitja.
___
Cython-dev mailing list
Cython-dev@codespeak.net
http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
On Fri, Dec 10, 2010 at 12:22 AM, Vitja Makarov wrote:
> 2010/12/10 Stefan Behnel :
>> Stefan Behnel, 09.12.2010 10:46:
>>> Vitja Makarov, 08.12.2010 22:45:
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.
>>>
>>> I'll take a close look ASAP.
>>>
- Temps are saved/restored/allocated inside YieldExprNode using
helper ClosureTempAllocator
>>
>> Ok, that currently lives in ParseTreeTransforms, which is totally the wrong
>> place for anything that generates code. It also mixes functionality that
>> normally lives in Symtab.py with functionality that lives in Code.py.
>>
>
> Yes, I know. That was placed there to give it a try (actually I'm not
> sure what is the best place for it). ClosureTempAllocator now is
> attribute of defnode and all yieldnodes, that's not clean.
>
> ExprNode.StopIterationNode isn't expression should it be moved to Nodes.py?
>
> create_abstract_generator() is rather dirty
>
> I don't know how to bind C functions to CClass.
>
> I think that generator body could have PyObject *body(PyObject *self,
> PyObject *value) signature
> With value=Null meaning exception.
>
>> Could you give me ("scoder", or better all Cython devs) commit rights to
>> your repo so that I can start refactoring the code and fix problems I find?
>>
>
> I've added you, but I don't know how to add all devs together.
I don't think there's a way to do that, just add whoever you want help
participating on this branch. My username is robertwb, and though I
haven't rally had time to read any code yet, I'm very interested and
would like to help out if I can.
>> I could also add Hudson jobs for it. Robert, would it be ok to add them for
>> all Python versions?
I think that would be excessive. Lets just do 2.4, 2.7, and a 3.x. If
there's a strange corner case that only manifests in, say, 2.5 then
we'll deal with it later.
> Yeah, please that would be nice.
>
- PyxGenerator methods are defined via declare_var
>>>
>>> Should we call that "CyGenerator" ? :)
>>
>> I actually meant to call it "CyGenerator" at the Python level, but I now
>> see that it's actually a purely internal thing. That makes __Pyx_Generator
>> a better choice, given that "__pyx_" is the globally reserved prefix in
>> Cython code.
>>
>
> Should rename it back ;)
>
>> Eventually, we may(!) want to reuse the superclass across modules, but I
>> currently have no idea how that could be made to work. So it's just another
>> thing that we don't need to worry about now.
>>
>
> Okay.
>
> How can I declare function entry with METH_VARGS signature?
> Using TypeSlots I can add METH_VARGS|METH_KWARGS entries but not
> single METH_VARGS
>
> That's preferd for "throw" method.
Just let it be METH_VARGS|METH_KWARGS for now. That doesn't mean it
can actually take arbitrary keyword arguments, just that the arguments
that are needed can be passed in as keywords.
- Robert
___
Cython-dev mailing list
Cython-dev@codespeak.net
http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
2010/12/10 Stefan Behnel :
> Stefan Behnel, 09.12.2010 10:46:
>> Vitja Makarov, 08.12.2010 22:45:
>>> 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.
>>
>> I'll take a close look ASAP.
>>
>>> - Temps are saved/restored/allocated inside YieldExprNode using
>>> helper ClosureTempAllocator
>
> Ok, that currently lives in ParseTreeTransforms, which is totally the wrong
> place for anything that generates code. It also mixes functionality that
> normally lives in Symtab.py with functionality that lives in Code.py.
>
Yes, I know. That was placed there to give it a try (actually I'm not
sure what is the best place for it). ClosureTempAllocator now is
attribute of defnode and all yieldnodes, that's not clean.
ExprNode.StopIterationNode isn't expression should it be moved to Nodes.py?
create_abstract_generator() is rather dirty
I don't know how to bind C functions to CClass.
I think that generator body could have PyObject *body(PyObject *self,
PyObject *value) signature
With value=Null meaning exception.
> Could you give me ("scoder", or better all Cython devs) commit rights to
> your repo so that I can start refactoring the code and fix problems I find?
>
I've added you, but I don't know how to add all devs together.
> I could also add Hudson jobs for it. Robert, would it be ok to add them for
> all Python versions?
>
>
Yeah, please that would be nice.
>>> - PyxGenerator methods are defined via declare_var
>>
>> Should we call that "CyGenerator" ? :)
>
> I actually meant to call it "CyGenerator" at the Python level, but I now
> see that it's actually a purely internal thing. That makes __Pyx_Generator
> a better choice, given that "__pyx_" is the globally reserved prefix in
> Cython code.
>
Should rename it back ;)
> Eventually, we may(!) want to reuse the superclass across modules, but I
> currently have no idea how that could be made to work. So it's just another
> thing that we don't need to worry about now.
>
Okay.
How can I declare function entry with METH_VARGS signature?
Using TypeSlots I can add METH_VARGS|METH_KWARGS entries but not
single METH_VARGS
That's preferd for "throw" method.
--
vitja.
___
Cython-dev mailing list
Cython-dev@codespeak.net
http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
Stefan Behnel, 09.12.2010 10:46:
> Vitja Makarov, 08.12.2010 22:45:
>> 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.
>
> I'll take a close look ASAP.
>
>>- Temps are saved/restored/allocated inside YieldExprNode using
>> helper ClosureTempAllocator
Ok, that currently lives in ParseTreeTransforms, which is totally the wrong
place for anything that generates code. It also mixes functionality that
normally lives in Symtab.py with functionality that lives in Code.py.
Could you give me ("scoder", or better all Cython devs) commit rights to
your repo so that I can start refactoring the code and fix problems I find?
I could also add Hudson jobs for it. Robert, would it be ok to add them for
all Python versions?
>>- PyxGenerator methods are defined via declare_var
>
> Should we call that "CyGenerator" ? :)
I actually meant to call it "CyGenerator" at the Python level, but I now
see that it's actually a purely internal thing. That makes __Pyx_Generator
a better choice, given that "__pyx_" is the globally reserved prefix in
Cython code.
Eventually, we may(!) want to reuse the superclass across modules, but I
currently have no idea how that could be made to work. So it's just another
thing that we don't need to worry about now.
Stefan
___
Cython-dev mailing list
Cython-dev@codespeak.net
http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
On Thu, Dec 9, 2010 at 12:45 PM, Vitja Makarov wrote: > 2010/12/9 Vitja Makarov : >> 2010/12/9 Robert Bradshaw : >>> On Thu, Dec 9, 2010 at 2:50 AM, Vitja Makarov >>> wrote: 2010/12/9 Robert Bradshaw : > On Thu, Dec 9, 2010 at 2:10 AM, Stefan Behnel wrote: >> Robert Bradshaw, 09.12.2010 11:00: >>> On Thu, Dec 9, 2010 at 1:46 AM, Stefan Behnel wrote: Vitja Makarov, 08.12.2010 22:45: > 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. *Way* cool, thanks! I'll take a close look ASAP. Please feel free to send in a new patch when you have it. If you use hg, you can use "hg bundle" to collect commit series. Don't know about git. BTW, why not set up a github branch for this? >>> >>> That's exactly what I was thinking. It'll make it easier to review as >>> well, and for others to contribute without disturbing the main line. >> >> BTW, when this goes in, the next Cython version to release is basically >> 1.0, right? There are some minor Python compatibility issues left, but >> that'll be the last major feature, the way I count it. > > This will be the last major feature before 1.0, but I don't know that > the first release this is in should be the 1.0 release. > > I also think we should set the compatibility bar a bit higher for 1.0 > as well--I want a concrete list of all the differences between Python > X.y and Cython (for un-annotated code) and to be able to say that, if > you don't count on the behavior in this list (e.g. identity vs > equality for floating point literals, maybe stack frames) then you > should be able to compile your code and have it just work, but > hopefully faster. At the very least, until we at least attempt to make > such a list, we won't know where we stand (the regression tests > presumably being somewhat of a starting point, though I'd like to get > all of them passing (unless they deal with implementation > details...)). > > - Robert > ___ > Cython-dev mailing list > Cython-dev@codespeak.net > http://codespeak.net/mailman/listinfo/cython-dev > >> - Comprehensions are now handled by OldYieldExprNode > >... another one of those bad names. ;) > >Any reason you couldn't reuse it for both? Didn't yet implemented ;) GenexpNode should be replaced with DefFunc one that's easy but I want to fix refnanny first. Then fix refcount leaks. >> - I don't know how to make refnanny work, should refnanny context be >> stored in closure? > > Good call. That's a tricky question. > > What about generating a GIVEREF for every Python temp you store in the > closure, a GOTREF for everything you take back out, and finish/restart the > refnanny context around the yield? Yes, that would help. But the main problem is refnanny context setupcontext defines local variable __pyx_refnanny, we should have one more macro refnanny_reinitalize_context() >>> >>> Yes, we'll need to split this up into two macros. This has actually >>> bitten be before, when I was writing the profiler. Did you know that ; >>> all by itself is considered a statement, hence one can't declare >>> variables after it in some strict compilers? >>> >> BTW, why not set up a github branch for this? > > That's exactly what I was thinking. It'll make it easier to review as > well, and for others to contribute without disturbing the main line. Should I fork cython from github? >>> >>> Yes, please do. >>> Is github insync with hg repo? >>> >>> I've been manually keeping the two in sync--just pushed now. >>> What is main repo now? >>> >>> It's a DVCS :). Some people have been developing on the git side of >>> things, and some on the hg side of things, and I've been manually >>> syncing the two now and then. Obviously this isn't a long term >>> solution, and I think setting up an automated synchronization both >>> ways would cause more confusion than benefit, so as soon as the >>> buildbot starts looking at github I'll make the hg ones read-only. >>> (Probably worth waiting until after the release to do this.) >>> ps: I think we should hurry with 1.0 release >>> >>> Yes, I'd like to hurry too, and with all the stuff you're doing it >>> should be soon :). I just think it's important to be able to say >>> "compiles all Python code" rather than "compiles most Python code" and >>> have a (hopefully small) concrete lits of well-delineated caveats. >>> >>> - Robert >>> ___ >>> Cython-dev mailing list >>> Cython-dev@codespeak.net >>> http://codespeak.net/mailman/listinfo/cython-dev >>> >> >> I've created github rep
Re: [Cython] General generator expressions
2010/12/9 Vitja Makarov : > 2010/12/9 Robert Bradshaw : >> On Thu, Dec 9, 2010 at 2:50 AM, Vitja Makarov >> wrote: >>> 2010/12/9 Robert Bradshaw : On Thu, Dec 9, 2010 at 2:10 AM, Stefan Behnel wrote: > Robert Bradshaw, 09.12.2010 11:00: >> On Thu, Dec 9, 2010 at 1:46 AM, Stefan Behnel wrote: >>> Vitja Makarov, 08.12.2010 22:45: 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. >>> >>> *Way* cool, thanks! I'll take a close look ASAP. Please feel free to >>> send >>> in a new patch when you have it. If you use hg, you can use "hg bundle" >>> to >>> collect commit series. Don't know about git. >>> >>> BTW, why not set up a github branch for this? >> >> That's exactly what I was thinking. It'll make it easier to review as >> well, and for others to contribute without disturbing the main line. > > BTW, when this goes in, the next Cython version to release is basically > 1.0, right? There are some minor Python compatibility issues left, but > that'll be the last major feature, the way I count it. This will be the last major feature before 1.0, but I don't know that the first release this is in should be the 1.0 release. I also think we should set the compatibility bar a bit higher for 1.0 as well--I want a concrete list of all the differences between Python X.y and Cython (for un-annotated code) and to be able to say that, if you don't count on the behavior in this list (e.g. identity vs equality for floating point literals, maybe stack frames) then you should be able to compile your code and have it just work, but hopefully faster. At the very least, until we at least attempt to make such a list, we won't know where we stand (the regression tests presumably being somewhat of a starting point, though I'd like to get all of them passing (unless they deal with implementation details...)). - Robert ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev >>> > - Comprehensions are now handled by OldYieldExprNode ... another one of those bad names. ;) Any reason you couldn't reuse it for both? >>> >>> Didn't yet implemented ;) GenexpNode should be replaced with DefFunc >>> one that's easy but I want to fix refnanny first. Then fix refcount >>> leaks. >>> > - I don't know how to make refnanny work, should refnanny context be > stored in closure? Good call. That's a tricky question. What about generating a GIVEREF for every Python temp you store in the closure, a GOTREF for everything you take back out, and finish/restart the refnanny context around the yield? >>> >>> Yes, that would help. But the main problem is refnanny context >>> setupcontext defines local variable __pyx_refnanny, we should have one >>> more macro refnanny_reinitalize_context() >> >> Yes, we'll need to split this up into two macros. This has actually >> bitten be before, when I was writing the profiler. Did you know that ; >> all by itself is considered a statement, hence one can't declare >> variables after it in some strict compilers? >> > BTW, why not set up a github branch for this? That's exactly what I was thinking. It'll make it easier to review as well, and for others to contribute without disturbing the main line. >>> >>> Should I fork cython from github? >> >> Yes, please do. >> >>> Is github insync with hg repo? >> >> I've been manually keeping the two in sync--just pushed now. >> >>> What is main repo now? >> >> It's a DVCS :). Some people have been developing on the git side of >> things, and some on the hg side of things, and I've been manually >> syncing the two now and then. Obviously this isn't a long term >> solution, and I think setting up an automated synchronization both >> ways would cause more confusion than benefit, so as soon as the >> buildbot starts looking at github I'll make the hg ones read-only. >> (Probably worth waiting until after the release to do this.) >> >>> ps: I think we should hurry with 1.0 release >> >> Yes, I'd like to hurry too, and with all the stuff you're doing it >> should be soon :). I just think it's important to be able to say >> "compiles all Python code" rather than "compiles most Python code" and >> have a (hopefully small) concrete lits of well-delineated caveats. >> >> - Robert >> ___ >> Cython-dev mailing list >> Cython-dev@codespeak.net >> http://codespeak.net/mailman/listinfo/cython-dev >> > > I've created github repo https://github.com/vitek/cython/ > > And changed __Pyx_Generator to __CyGenerator... > > -- > vitja. > I've fixed refnanny. Now regr are okay: -
Re: [Cython] General generator expressions
2010/12/9 Robert Bradshaw : > On Thu, Dec 9, 2010 at 2:50 AM, Vitja Makarov wrote: >> 2010/12/9 Robert Bradshaw : >>> On Thu, Dec 9, 2010 at 2:10 AM, Stefan Behnel wrote: Robert Bradshaw, 09.12.2010 11:00: > On Thu, Dec 9, 2010 at 1:46 AM, Stefan Behnel wrote: >> Vitja Makarov, 08.12.2010 22:45: >>> 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. >> >> *Way* cool, thanks! I'll take a close look ASAP. Please feel free to send >> in a new patch when you have it. If you use hg, you can use "hg bundle" >> to >> collect commit series. Don't know about git. >> >> BTW, why not set up a github branch for this? > > That's exactly what I was thinking. It'll make it easier to review as > well, and for others to contribute without disturbing the main line. BTW, when this goes in, the next Cython version to release is basically 1.0, right? There are some minor Python compatibility issues left, but that'll be the last major feature, the way I count it. >>> >>> This will be the last major feature before 1.0, but I don't know that >>> the first release this is in should be the 1.0 release. >>> >>> I also think we should set the compatibility bar a bit higher for 1.0 >>> as well--I want a concrete list of all the differences between Python >>> X.y and Cython (for un-annotated code) and to be able to say that, if >>> you don't count on the behavior in this list (e.g. identity vs >>> equality for floating point literals, maybe stack frames) then you >>> should be able to compile your code and have it just work, but >>> hopefully faster. At the very least, until we at least attempt to make >>> such a list, we won't know where we stand (the regression tests >>> presumably being somewhat of a starting point, though I'd like to get >>> all of them passing (unless they deal with implementation >>> details...)). >>> >>> - Robert >>> ___ >>> Cython-dev mailing list >>> Cython-dev@codespeak.net >>> http://codespeak.net/mailman/listinfo/cython-dev >>> >> - Comprehensions are now handled by OldYieldExprNode >>> >>>... another one of those bad names. ;) >>> >>>Any reason you couldn't reuse it for both? >> >> Didn't yet implemented ;) GenexpNode should be replaced with DefFunc >> one that's easy but I want to fix refnanny first. Then fix refcount >> leaks. >> - I don't know how to make refnanny work, should refnanny context be stored in closure? >>> >>> Good call. That's a tricky question. >>> >>> What about generating a GIVEREF for every Python temp you store in the >>> closure, a GOTREF for everything you take back out, and finish/restart the >>> refnanny context around the yield? >> >> Yes, that would help. But the main problem is refnanny context >> setupcontext defines local variable __pyx_refnanny, we should have one >> more macro refnanny_reinitalize_context() > > Yes, we'll need to split this up into two macros. This has actually > bitten be before, when I was writing the profiler. Did you know that ; > all by itself is considered a statement, hence one can't declare > variables after it in some strict compilers? > BTW, why not set up a github branch for this? >>> >>> That's exactly what I was thinking. It'll make it easier to review as >>> well, and for others to contribute without disturbing the main line. >> >> Should I fork cython from github? > > Yes, please do. > >> Is github insync with hg repo? > > I've been manually keeping the two in sync--just pushed now. > >> What is main repo now? > > It's a DVCS :). Some people have been developing on the git side of > things, and some on the hg side of things, and I've been manually > syncing the two now and then. Obviously this isn't a long term > solution, and I think setting up an automated synchronization both > ways would cause more confusion than benefit, so as soon as the > buildbot starts looking at github I'll make the hg ones read-only. > (Probably worth waiting until after the release to do this.) > >> ps: I think we should hurry with 1.0 release > > Yes, I'd like to hurry too, and with all the stuff you're doing it > should be soon :). I just think it's important to be able to say > "compiles all Python code" rather than "compiles most Python code" and > have a (hopefully small) concrete lits of well-delineated caveats. > > - Robert > ___ > Cython-dev mailing list > Cython-dev@codespeak.net > http://codespeak.net/mailman/listinfo/cython-dev > I've created github repo https://github.com/vitek/cython/ And changed __Pyx_Generator to __CyGenerator... -- vitja. ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
On Thu, Dec 9, 2010 at 2:50 AM, Vitja Makarov wrote: > 2010/12/9 Robert Bradshaw : >> On Thu, Dec 9, 2010 at 2:10 AM, Stefan Behnel wrote: >>> Robert Bradshaw, 09.12.2010 11:00: On Thu, Dec 9, 2010 at 1:46 AM, Stefan Behnel wrote: > Vitja Makarov, 08.12.2010 22:45: >> 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. > > *Way* cool, thanks! I'll take a close look ASAP. Please feel free to send > in a new patch when you have it. If you use hg, you can use "hg bundle" to > collect commit series. Don't know about git. > > BTW, why not set up a github branch for this? That's exactly what I was thinking. It'll make it easier to review as well, and for others to contribute without disturbing the main line. >>> >>> BTW, when this goes in, the next Cython version to release is basically >>> 1.0, right? There are some minor Python compatibility issues left, but >>> that'll be the last major feature, the way I count it. >> >> This will be the last major feature before 1.0, but I don't know that >> the first release this is in should be the 1.0 release. >> >> I also think we should set the compatibility bar a bit higher for 1.0 >> as well--I want a concrete list of all the differences between Python >> X.y and Cython (for un-annotated code) and to be able to say that, if >> you don't count on the behavior in this list (e.g. identity vs >> equality for floating point literals, maybe stack frames) then you >> should be able to compile your code and have it just work, but >> hopefully faster. At the very least, until we at least attempt to make >> such a list, we won't know where we stand (the regression tests >> presumably being somewhat of a starting point, though I'd like to get >> all of them passing (unless they deal with implementation >> details...)). >> >> - Robert >> ___ >> Cython-dev mailing list >> Cython-dev@codespeak.net >> http://codespeak.net/mailman/listinfo/cython-dev >> > >>> - Comprehensions are now handled by OldYieldExprNode >> >>... another one of those bad names. ;) >> >>Any reason you couldn't reuse it for both? > > Didn't yet implemented ;) GenexpNode should be replaced with DefFunc > one that's easy but I want to fix refnanny first. Then fix refcount > leaks. > >>> - I don't know how to make refnanny work, should refnanny context be >>> stored in closure? >> >> Good call. That's a tricky question. >> >> What about generating a GIVEREF for every Python temp you store in the >> closure, a GOTREF for everything you take back out, and finish/restart the >> refnanny context around the yield? > > Yes, that would help. But the main problem is refnanny context > setupcontext defines local variable __pyx_refnanny, we should have one > more macro refnanny_reinitalize_context() Yes, we'll need to split this up into two macros. This has actually bitten be before, when I was writing the profiler. Did you know that ; all by itself is considered a statement, hence one can't declare variables after it in some strict compilers? >>> BTW, why not set up a github branch for this? >> >> That's exactly what I was thinking. It'll make it easier to review as >> well, and for others to contribute without disturbing the main line. > > Should I fork cython from github? Yes, please do. > Is github insync with hg repo? I've been manually keeping the two in sync--just pushed now. > What is main repo now? It's a DVCS :). Some people have been developing on the git side of things, and some on the hg side of things, and I've been manually syncing the two now and then. Obviously this isn't a long term solution, and I think setting up an automated synchronization both ways would cause more confusion than benefit, so as soon as the buildbot starts looking at github I'll make the hg ones read-only. (Probably worth waiting until after the release to do this.) > ps: I think we should hurry with 1.0 release Yes, I'd like to hurry too, and with all the stuff you're doing it should be soon :). I just think it's important to be able to say "compiles all Python code" rather than "compiles most Python code" and have a (hopefully small) concrete lits of well-delineated caveats. - Robert ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
2010/12/9 Robert Bradshaw : > On Thu, Dec 9, 2010 at 2:10 AM, Stefan Behnel wrote: >> Robert Bradshaw, 09.12.2010 11:00: >>> On Thu, Dec 9, 2010 at 1:46 AM, Stefan Behnel wrote: Vitja Makarov, 08.12.2010 22:45: > 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. *Way* cool, thanks! I'll take a close look ASAP. Please feel free to send in a new patch when you have it. If you use hg, you can use "hg bundle" to collect commit series. Don't know about git. BTW, why not set up a github branch for this? >>> >>> That's exactly what I was thinking. It'll make it easier to review as >>> well, and for others to contribute without disturbing the main line. >> >> BTW, when this goes in, the next Cython version to release is basically >> 1.0, right? There are some minor Python compatibility issues left, but >> that'll be the last major feature, the way I count it. > > This will be the last major feature before 1.0, but I don't know that > the first release this is in should be the 1.0 release. > > I also think we should set the compatibility bar a bit higher for 1.0 > as well--I want a concrete list of all the differences between Python > X.y and Cython (for un-annotated code) and to be able to say that, if > you don't count on the behavior in this list (e.g. identity vs > equality for floating point literals, maybe stack frames) then you > should be able to compile your code and have it just work, but > hopefully faster. At the very least, until we at least attempt to make > such a list, we won't know where we stand (the regression tests > presumably being somewhat of a starting point, though I'd like to get > all of them passing (unless they deal with implementation > details...)). > > - Robert > ___ > Cython-dev mailing list > Cython-dev@codespeak.net > http://codespeak.net/mailman/listinfo/cython-dev > >> - Comprehensions are now handled by OldYieldExprNode > >... another one of those bad names. ;) > >Any reason you couldn't reuse it for both? Didn't yet implemented ;) GenexpNode should be replaced with DefFunc one that's easy but I want to fix refnanny first. Then fix refcount leaks. >> - I don't know how to make refnanny work, should refnanny context be >> stored in closure? > > Good call. That's a tricky question. > > What about generating a GIVEREF for every Python temp you store in the > closure, a GOTREF for everything you take back out, and finish/restart the > refnanny context around the yield? Yes, that would help. But the main problem is refnanny context setupcontext defines local variable __pyx_refnanny, we should have one more macro refnanny_reinitalize_context() >> BTW, why not set up a github branch for this? > > That's exactly what I was thinking. It'll make it easier to review as > well, and for others to contribute without disturbing the main line. Should I fork cython from github? Is github insync with hg repo? What is main repo now? ps: I think we should horry with 1.0 release -- vitja. ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
On Thu, Dec 9, 2010 at 2:10 AM, Stefan Behnel wrote: > Robert Bradshaw, 09.12.2010 11:00: >> On Thu, Dec 9, 2010 at 1:46 AM, Stefan Behnel wrote: >>> Vitja Makarov, 08.12.2010 22:45: 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. >>> >>> *Way* cool, thanks! I'll take a close look ASAP. Please feel free to send >>> in a new patch when you have it. If you use hg, you can use "hg bundle" to >>> collect commit series. Don't know about git. >>> >>> BTW, why not set up a github branch for this? >> >> That's exactly what I was thinking. It'll make it easier to review as >> well, and for others to contribute without disturbing the main line. > > BTW, when this goes in, the next Cython version to release is basically > 1.0, right? There are some minor Python compatibility issues left, but > that'll be the last major feature, the way I count it. This will be the last major feature before 1.0, but I don't know that the first release this is in should be the 1.0 release. I also think we should set the compatibility bar a bit higher for 1.0 as well--I want a concrete list of all the differences between Python X.y and Cython (for un-annotated code) and to be able to say that, if you don't count on the behavior in this list (e.g. identity vs equality for floating point literals, maybe stack frames) then you should be able to compile your code and have it just work, but hopefully faster. At the very least, until we at least attempt to make such a list, we won't know where we stand (the regression tests presumably being somewhat of a starting point, though I'd like to get all of them passing (unless they deal with implementation details...)). - Robert ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
Robert Bradshaw, 09.12.2010 11:00: > On Thu, Dec 9, 2010 at 1:46 AM, Stefan Behnel wrote: >> Vitja Makarov, 08.12.2010 22:45: >>> 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. >> >> *Way* cool, thanks! I'll take a close look ASAP. Please feel free to send >> in a new patch when you have it. If you use hg, you can use "hg bundle" to >> collect commit series. Don't know about git. >> >> BTW, why not set up a github branch for this? > > That's exactly what I was thinking. It'll make it easier to review as > well, and for others to contribute without disturbing the main line. BTW, when this goes in, the next Cython version to release is basically 1.0, right? There are some minor Python compatibility issues left, but that'll be the last major feature, the way I count it. Stefan ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
On Thu, Dec 9, 2010 at 1:46 AM, Stefan Behnel wrote: > Vitja Makarov, 08.12.2010 22:45: >> 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. > > *Way* cool, thanks! I'll take a close look ASAP. Please feel free to send > in a new patch when you have it. If you use hg, you can use "hg bundle" to > collect commit series. Don't know about git. > > BTW, why not set up a github branch for this? That's exactly what I was thinking. It'll make it easier to review as well, and for others to contribute without disturbing the main line. >> - Temps are saved/restored/allocated inside YieldExprNode using >> helper ClosureTempAllocator >> - PyxGenerator methods are defined via declare_var > > Should we call that "CyGenerator" ? :) > > >> - Send, __next__(), __iter__() are implemented while close(), throw() >> aren't >> - YieldExprNode doesn't handle exceptions > > Ok, sure, that's why close() and throw() don't work yet. I think that's > fine for now. The first three are the most interesting ones. However, > adding exception support shouldn't be hard at all, AFAICT, but they may > benefit from a cdef function for the generator body in order to properly > pass in the exception propagation trigger. > > >> - I don't know how to make refnanny work, should refnanny context be >> stored in closure? > > Good call. That's a tricky question. > > What about generating a GIVEREF for every Python temp you store in the > closure, a GOTREF for everything you take back out, and finish/restart the > refnanny context around the yield? That's what I would do as well. - Robert ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
Vitja Makarov, 08.12.2010 22:45: > 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. *Way* cool, thanks! I'll take a close look ASAP. Please feel free to send in a new patch when you have it. If you use hg, you can use "hg bundle" to collect commit series. Don't know about git. BTW, why not set up a github branch for this? > - Temps are saved/restored/allocated inside YieldExprNode using > helper ClosureTempAllocator > - PyxGenerator methods are defined via declare_var Should we call that "CyGenerator" ? :) > - Send, __next__(), __iter__() are implemented while close(), throw() > aren't > - YieldExprNode doesn't handle exceptions Ok, sure, that's why close() and throw() don't work yet. I think that's fine for now. The first three are the most interesting ones. However, adding exception support shouldn't be hard at all, AFAICT, but they may benefit from a cdef function for the generator body in order to properly pass in the exception propagation trigger. > - I don't know how to make refnanny work, should refnanny context be > stored in closure? Good call. That's a tricky question. What about generating a GIVEREF for every Python temp you store in the closure, a GOTREF for everything you take back out, and finish/restart the refnanny context around the yield? > - Comprehensions are now handled by OldYieldExprNode ... another one of those bad names. ;) Any reason you couldn't reuse it for both? Stefan ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
2010/12/8 Vitja Makarov :
> 2010/12/7 Vitja Makarov :
>> 2010/12/7 Vitja Makarov :
>>> 2010/12/7 Robert Bradshaw :
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
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,
>>> visib
Re: [Cython] General generator expressions
2010/12/7 Vitja Makarov :
> 2010/12/7 Vitja Makarov :
>> 2010/12/7 Robert Bradshaw :
>>> 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
>>> 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
Re: [Cython] General generator expressions
2010/12/7 Vitja Makarov :
> 2010/12/7 Robert Bradshaw :
>> 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
>> 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 think
Re: [Cython] General generator expressions
2010/12/7 Robert Bradshaw :
> 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
> 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 *k
Re: [Cython] General generator expressions
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 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
Re: [Cython] General generator expressions
Stefan Behnel, 06.12.2010 11:35: > I think it would be best to generate the straight assignment code from > within of the YieldExprNode: > > closure->place_for_int_temp[0] = __pyx_t_2 > closure->place_for_int_temp[1] = __pyx_t_5 > closure->place_for_object_temp[0] = __pyx_t_9 > return result > resume: > __pyx_t_2 = closure->place_for_int_temp[0] > ... > > For starters, I wouldn't mind if the closure fields were all "size_t" and > the assignments just did an ugly cast on the RHS. Might be easier than > properly allocating separate arrays of correctly sized fields. ... although, of course, not every C value fits into size_t, either. There may be structs being passed around by value, for example. Tricky... Hmm, maybe it's really ok to start with temps in the closure for now, just like we accepted that closures were huge until they were cut down to what was actually needed. I think the feature is more important than the optimisations around it. Stefan ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
On 12/06/2010 11:35 AM, Stefan Behnel wrote: > Vitja Makarov, 06.12.2010 10:56: > >> 2010/12/6 Stefan Behnel: >> >>> Vitja Makarov, 06.12.2010 09:56: >>> 2010/12/6 Stefan Behnel: > Vitja Makarov, 05.12.2010 20:12: > >> 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. >>> But it only has to be done when suspending/resuming, not every time a temp >>> is used. Given that there is quite some overhead involved in executing a >>> yield/resume cycle, it doesn't matter if we add a couple of fast C >>> assignments to that. >>> >>> 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, *much* slower. Temps are not meant to live on the stack. Most of the >>> time, they will just live in a CPU register, especially on x86_64 and other >>> architectures with plenty of registers. When you put them into the closure, >>> the C compiler must assume that it's an external operation when accessing >>> them and can't know that the next time it reads the variable it still has >>> the same value that it just wrote. That's kills tons of possible C >>> optimisations. >>> >> Hmm. It is not really necessery to check each for value each time, >> it's not defined as volatile. >> > My knowledge of C is admittedly limited. Why should the C compiler be > allowed to assume that a value stored somewhere in a foreign memory > location doesn't change its value between separate store and read operations? > Unless you call another function, the C compiler will know just from the actions of the executing thread. (You can't use a variable/struct field to e.g. communicate between threads...unless perhaps if you mark it volatile, but one should use locking instead). But, of course, the closure may well call other functions in libraries where the C compiler has no access. So I believe Stefan is right. It corresponds to providing hints to the compiler that "no external code will modify this memory". Dag Sverre ___ Cython-dev mailing list Cython-dev@codespeak.net http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
Vitja Makarov, 06.12.2010 10:56:
> 2010/12/6 Stefan Behnel:
>> Vitja Makarov, 06.12.2010 09:56:
>>> 2010/12/6 Stefan Behnel:
Vitja Makarov, 05.12.2010 20:12:
> 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.
>>
>> But it only has to be done when suspending/resuming, not every time a temp
>> is used. Given that there is quite some overhead involved in executing a
>> yield/resume cycle, it doesn't matter if we add a couple of fast C
>> assignments to that.
>>
>>> 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, *much* slower. Temps are not meant to live on the stack. Most of the
>> time, they will just live in a CPU register, especially on x86_64 and other
>> architectures with plenty of registers. When you put them into the closure,
>> the C compiler must assume that it's an external operation when accessing
>> them and can't know that the next time it reads the variable it still has
>> the same value that it just wrote. That's kills tons of possible C
>> optimisations.
>
> Hmm. It is not really necessery to check each for value each time,
> it's not defined as volatile.
My knowledge of C is admittedly limited. Why should the C compiler be
allowed to assume that a value stored somewhere in a foreign memory
location doesn't change its value between separate store and read operations?
> Compiler will try to put it in registers and write back when reqired.
The difference is, when it's not in the closure, the function body (or
current C scope) owns it, so the C compiler doesn't have to generate code
to write it anywhere in the first place. Most temps are really just there
for convenience when generating the C code, not because there really is
something to store away. The C compiler is expected to drop almost all of them.
> Btw it's better to try both ways and choose the fastest one.
I know, predicting performance is hard at best, but in this case, it's
clear enough to me that letting all temps live in the closure suffers from
two serious drawbacks: space *and* time.
> How should save/restore code look like?
>
> PyObject *all_temps[NUMBER_OF_PYOBJECT_TEMPS];
>
> // save
> memcpy(closure->all_temps, all_temps, sizeof(all_temps));?
>
> Or:
>
> for (int i=0; i< NUMBER_OF_PYOBJECT_TEMPS; i++) {
> if (all_temps[i]) {
> // this will create tuple (n, all_temps[i]) and add it to
> closure->temps_tuple
> add_temp_to_closure(closure, i, all_temps[i]);
> }
> }
>
> How should it handle native C types?
The CEP has some comments on this.
I think it would be best to generate the straight assignment code from
within of the YieldExprNode:
closure->place_for_int_temp[0] = __pyx_t_2
closure->place_for_int_temp[1] = __pyx_t_5
closure->place_for_object_temp[0] = __pyx_t_9
return result
resume:
__pyx_t_2 = closure->place_for_int_temp[0]
...
For starters, I wouldn't mind if the closure fields were all "size_t" and
the assignments just did an ugly cast on the RHS. Might be easier than
properly allocating separate arrays of correctly sized fields.
BTW, I just noticed that this won't work with exception handling yet. In
try-except and try-finally statements, we keep the current exception state
in block local variables, those would have to get stored away as well.
However, the problem is that this can be in nested block scopes, so the
outer variables may not even be available to the C code. Another thing to
deal with once this works...
Stefan
___
Cython-dev mailing list
Cython-dev@codespeak.net
http://codespeak.net/mailman
Re: [Cython] General generator expressions
2010/12/6 Stefan Behnel :
> Vitja Makarov, 06.12.2010 09:56:
>> 2010/12/6 Stefan Behnel:
>>> Vitja Makarov, 05.12.2010 20:12:
def my_generator(self, args, kwargs):
closure = my_generator_closure_new()
closure.is_running = 0
closure.resume_label = 0
closure.args = args
closure.kwargs = kwargs
closure.outer_scope = self
closure.generator_body = my_generator_body
return closure
generator body:
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.
>
> Generators, yes. Generator expressions, no. I've written in the CEP that it
> would be nice to start with splitting the way Python functions work. I
> don't ask you to do this (certainly makes a nice optimisation when the
> functionality itself is done), but it would easily enable a cdef function
> here, which would hugely reduce the burden when advancing a generator.
>
Yeah. Splitting would be nice. But I don't know yet how to do it the best way.
So it could wait a little.
>
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.
>
> But it only has to be done when suspending/resuming, not every time a temp
> is used. Given that there is quite some overhead involved in executing a
> yield/resume cycle, it doesn't matter if we add a couple of fast C
> assignments to that.
>
>
>> 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, *much* slower. Temps are not meant to live on the stack. Most of the
> time, they will just live in a CPU register, especially on x86_64 and other
> architectures with plenty of registers. When you put them into the closure,
> the C compiler must assume that it's an external operation when accessing
> them and can't know that the next time it reads the variable it still has
> the same value that it just wrote. That's kills tons of possible C
> optimisations.
>
>
Hmm. It is not really necessery to check each for value each time,
it's not defined as volatile.
Compiler will try to put it in registers and write back when reqired.
Btw it's better to try both ways and choose the fastest one.
How should save/restore code look like?
PyObject *all_temps[NUMBER_OF_PYOBJECT_TEMPS];
// save
memcpy(closure->all_temps, all_temps, sizeof(all_temps));?
Or:
for (int i=0; i < NUMBER_OF_PYOBJECT_TEMPS; i++) {
if (all_temps[i]) {
// this will create tuple (n, all_temps[i]) and add it to
closure->temps_tuple
add_temp_to_closure(closure, i, all_temps[i]);
}
}
How should it handle native C types?
struct closure {
/* abstract_generator */
PY_OBJECT_HEAD
/* generator internal vars */
PyObject *(*body)(struct closure *, PyObject *, int);
int resume_label;
/* Closure stuff */
} ;
and generator utility function will work like this:
PyObject *abstract_generator_next(PyObject *self, PyObject *args)
{
struct abstract_generator *generator = (struct abstract_generator *)
self;
PyObject *retval;
// check running
retval = generator->body(self, None, 0);
// unlock running
return retval;
}
But this way should be much better, don't know if that is possible btw.
struct closure {
struct abstract_generator generator;
/* closure stuff he
Re: [Cython] General generator expressions
Vitja Makarov, 06.12.2010 09:56:
> 2010/12/6 Stefan Behnel:
>> Vitja Makarov, 05.12.2010 20:12:
>>> def my_generator(self, args, kwargs):
>>> closure = my_generator_closure_new()
>>> closure.is_running = 0
>>> closure.resume_label = 0
>>> closure.args = args
>>> closure.kwargs = kwargs
>>> closure.outer_scope = self
>>> closure.generator_body = my_generator_body
>>> return closure
>>>
>>> generator body:
>>>
>>> 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.
Generators, yes. Generator expressions, no. I've written in the CEP that it
would be nice to start with splitting the way Python functions work. I
don't ask you to do this (certainly makes a nice optimisation when the
functionality itself is done), but it would easily enable a cdef function
here, which would hugely reduce the burden when advancing a generator.
>>> 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.
But it only has to be done when suspending/resuming, not every time a temp
is used. Given that there is quite some overhead involved in executing a
yield/resume cycle, it doesn't matter if we add a couple of fast C
assignments to that.
> 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, *much* slower. Temps are not meant to live on the stack. Most of the
time, they will just live in a CPU register, especially on x86_64 and other
architectures with plenty of registers. When you put them into the closure,
the C compiler must assume that it's an external operation when accessing
them and can't know that the next time it reads the variable it still has
the same value that it just wrote. That's kills tons of possible C
optimisations.
>>> struct closure {
>>> /* abstract_generator */
>>> PY_OBJECT_HEAD
>>> /* generator internal vars */
>>> PyObject *(*body)(struct closure *, PyObject *, int);
>>> int resume_label;
>>>
>>> /* Closure stuff */
>>> } ;
>>>
>>> and generator utility function will work like this:
>>>
>>> PyObject *abstract_generator_next(PyObject *self, PyObject *args)
>>> {
>>> struct abstract_generator *generator = (struct abstract_generator *)
>>> self;
>>> PyObject *retval;
>>> // check running
>>> retval = generator->body(self, None, 0);
>>> // unlock running
>>> return retval;
>>> }
>>>
>>> But this way should be much better, don't know if that is possible btw.
>>>
>>> struct closure {
>>> struct abstract_generator generator;
>>> /* closure stuff here */
>>> ...
>>> } ;
>>
>> That's called inheritance in Cython. Just declare a subtype of the specific
>> closure class.
>
> That's not inheritance exactly I don't want to call all cython&
> python stuff for base class (why not btw?)
Right, why not? Closures are instantiated by calling tp_new() directly, so
even the overhead of creating them as a subtype is pretty low. Plus, it
only has to be done once when instantiating the generator, not every time
it advances. So I consider it totally negligible over the overall lifetime.
> Just want to have some fields (now I think that is_running and body is
> enough) on top of closure stuff, and I don't want to declare things
> twice.
Then inheritance is your friend. It will create a vtable, though, if you
use methods.
>>> I add MarkGenerator visitor, that sets is_generator flag on DefNode
>>> and checks for returns/yields.
Re: [Cython] General generator expressions
2010/12/6 Stefan Behnel :
> Vitja Makarov, 05.12.2010 20:12:
>> I've started woring on generators on friday.
>
> Cool. This sounds like a larger code change, though. Try not to submit it
> as a single patch as that would likely be hard to review. Instead, put up a
> repo somewhere that others can look at, or submit bundles (as hg calls
> them) of separate commits.
>
>
Ok, I'll try :)
>> Now it can generate some code, but it doesn't work yet, what is done:
>> - switch() for resuming and argument parsing
>> - yield stuff
>> - closure temp allocation
>>
>> I'm going to put all the generator stuff into closure
>>
>> Generator function:
>>
>> Creates closure with all temps, args, variables, and all the internal stuff:
>> Also closure have all the required generator methods __iter__(),
>> __nextiter__() and so on.
>
> Ok, why not.
>
>
>> def my_generator(self, args, kwargs):
>> closure = my_generator_closure_new()
>> closure.is_running = 0
>> closure.resume_label = 0
>> closure.args = args
>> closure.kwargs = kwargs
>> closure.outer_scope = self
>> closure.generator_body = my_generator_body
>> return closure
>>
>> generator body:
>>
>> 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.
>
>
>> # 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.
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?
>> 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.
>
>
>> TODO:
>> - create C function that returns closure
>> - generator utility code
>
> At least for generator expressions, the latter wouldn't be needed for now.
>
>
>> I have problem with closures and generators.
>>
>> Now I want to implement closure structure this way:
>> struct abstract_generator {
>> PY_OBJECT_HEAD
>> /* generator internal vars */
>> PyObject *(*body)(struct closure *, PyObject *, int);
>> int resume_label;
>> }
>
> I hope you're not trying to do it manually at that level, are you?
> Declaring the fields on the closure class should just work.
>
No, I declare things using scope_class.declare_var ;)
>
>> struct closure {
>> /* abstract_generator */
>> PY_OBJECT_HEAD
>> /* generator internal vars */
>> PyObject *(*body)(struct closure *, PyObject *, int);
>> int resume_label;
>>
>> /* Closure stuff */
>> } ;
>>
>> and generator utility function will work like this:
>>
>> PyObject *abstract_generator_next(PyObject *self, PyObject *args)
>> {
>> struct abstract_generator *generator = (struct abstract_generator *)
>> self;
>> PyObject *retval;
>> // che
Re: [Cython] General generator expressions
Stefan Behnel, 06.12.2010 08:33:
> Vitja Makarov, 05.12.2010 20:12:
>> I have problem with closures and generators.
>>
>> Now I want to implement closure structure this way:
>> struct abstract_generator {
>> PY_OBJECT_HEAD
>> /* generator internal vars */
>> PyObject *(*body)(struct closure *, PyObject *, int);
>> int resume_label;
>> }
>
> I hope you're not trying to do it manually at that level, are you?
> Declaring the fields on the closure class should just work.
>
>
>> struct closure {
>> /* abstract_generator */
>> PY_OBJECT_HEAD
>> /* generator internal vars */
>> PyObject *(*body)(struct closure *, PyObject *, int);
>> int resume_label;
>>
>> /* Closure stuff */
>> } ;
>>
>> and generator utility function will work like this:
>>
>> PyObject *abstract_generator_next(PyObject *self, PyObject *args)
>> {
>> struct abstract_generator *generator = (struct abstract_generator *)
>> self;
>> PyObject *retval;
>> // check running
>> retval = generator->body(self, None, 0);
>> // unlock running
>> return retval;
>> }
>>
>> But this way should be much better, don't know if that is possible btw.
>>
>> struct closure {
>> struct abstract_generator generator;
>> /* closure stuff here */
>> ...
>> } ;
>
> That's called inheritance in Cython. Just declare a subtype of the specific
> closure class.
Erm, sorry, the other way round. Yes, I think it's a good idea to let the
closure class inherit from the abstract generator.
Stefan
___
Cython-dev mailing list
Cython-dev@codespeak.net
http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
Vitja Makarov, 05.12.2010 20:12:
> I've started woring on generators on friday.
Cool. This sounds like a larger code change, though. Try not to submit it
as a single patch as that would likely be hard to review. Instead, put up a
repo somewhere that others can look at, or submit bundles (as hg calls
them) of separate commits.
> Now it can generate some code, but it doesn't work yet, what is done:
> - switch() for resuming and argument parsing
> - yield stuff
> - closure temp allocation
>
> I'm going to put all the generator stuff into closure
>
> Generator function:
>
> Creates closure with all temps, args, variables, and all the internal stuff:
> Also closure have all the required generator methods __iter__(),
> __nextiter__() and so on.
Ok, why not.
> def my_generator(self, args, kwargs):
> closure = my_generator_closure_new()
> closure.is_running = 0
> closure.resume_label = 0
> closure.args = args
> closure.kwargs = kwargs
> closure.outer_scope = self
> closure.generator_body = my_generator_body
> return closure
>
> generator body:
>
> def my_generator_body(closure, value, is_exception):
Shouldn't that be a cdef function?
> # 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'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.
> TODO:
> - create C function that returns closure
> - generator utility code
At least for generator expressions, the latter wouldn't be needed for now.
> I have problem with closures and generators.
>
> Now I want to implement closure structure this way:
> struct abstract_generator {
> PY_OBJECT_HEAD
> /* generator internal vars */
> PyObject *(*body)(struct closure *, PyObject *, int);
> int resume_label;
> }
I hope you're not trying to do it manually at that level, are you?
Declaring the fields on the closure class should just work.
> struct closure {
> /* abstract_generator */
> PY_OBJECT_HEAD
> /* generator internal vars */
> PyObject *(*body)(struct closure *, PyObject *, int);
> int resume_label;
>
> /* Closure stuff */
> } ;
>
> and generator utility function will work like this:
>
> PyObject *abstract_generator_next(PyObject *self, PyObject *args)
> {
> struct abstract_generator *generator = (struct abstract_generator *)
> self;
> PyObject *retval;
> // check running
> retval = generator->body(self, None, 0);
> // unlock running
> return retval;
> }
>
> But this way should be much better, don't know if that is possible btw.
>
> struct closure {
> struct abstract_generator generator;
> /* closure stuff here */
> ...
> } ;
That's called inheritance in Cython. Just declare a subtype of the specific
closure class.
> I add MarkGenerator visitor, that sets is_generator flag on DefNode
> and checks for returns/yields.
Why do you need a new class here? Can't the MarkClosureVisitor handle this?
It's not like transform runs over the whole AST come for free...
Stefan
___
Cython-dev mailing list
Cython-dev@codespeak.net
http://codespeak.net/mailman/listinfo/cython-dev
Re: [Cython] General generator expressions
2010/12/5 Stefan Behnel :
> Hi,
>
> I just noticed that Vitja's closure refactoring brought us closer to
> supporting generator expressions (before supporting generators and
> coroutines). Here's what's missing:
>
> 1) transform a GeneratorExpressionNode into a DefNode or a subclass
> (obviously excluding inlined generator expressions)
>
> 2) generate the resume code at the start of the function body
>
> 3) store temps in closures. For simplicity, this can be done with a tuple
> (which supports NULL values to a certain extent). That way, we do not need
> to declare each temp separately as a closure field. The remaining tricky
> bit is then to figure out the maximum number of temps that are alive at a
> YieldExprNode (ok, that's actually easy) and to pass that number on to the
> closure class that needs to allocate a sufficiently large tuple.
>
> That doesn't sound too hard.
>
> The generators CEP has the details for each step.
>
> http://wiki.cython.org/enhancements/generators
>
> Going for generator expressions before implementing generators has the
> advantage that it gives us most of the infrastructure without requiring the
> additional steps of passing initial arguments into the generator and
> implementing the complete coroutine protocol (the yield nodes cannot return
> values, for example).
>
> Anyone with a little time to spare?
>
> Stefan
> ___
> Cython-dev mailing list
> Cython-dev@codespeak.net
> http://codespeak.net/mailman/listinfo/cython-dev
>
Hi!
I've started woring on generators on friday.
Now it can generate some code, but it doesn't work yet, what is done:
- switch() for resuming and argument parsing
- yield stuff
- closure temp allocation
I'm going to put all the generator stuff into closure
Generator function:
Creates closure with all temps, args, variables, and all the internal stuff:
Also closure have all the required generator methods __iter__(),
__nextiter__() and so on.
def my_generator(self, args, kwargs):
closure = my_generator_closure_new()
closure.is_running = 0
closure.resume_label = 0
closure.args = args
closure.kwargs = kwargs
closure.outer_scope = self
closure.generator_body = my_generator_body
return closure
generator body:
def my_generator_body(closure, value, is_exception):
# switch case
switch (closure.resume_lable):
case 0:
goto parse_arguments;
case 1:
goto resume_from_yield1;
parse_arguments:
# function body here
About temps:
now temps are allocated in closure using declare_var() and it works fine.
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?)
I'm not sure that tuple should be better for temps.
TODO:
- create C function that returns closure
- generator utility code
I have problem with closures and generators.
Now I want to implement closure structure this way:
struct abstract_generator {
PY_OBJECT_HEAD
/* generator internal vars */
PyObject *(*body)(struct closure *, PyObject *, int);
int resume_label;
}
struct closure {
/* abstract_generator */
PY_OBJECT_HEAD
/* generator internal vars */
PyObject *(*body)(struct closure *, PyObject *, int);
int resume_label;
/* Closure stuff */
} ;
and generator utility function will work like this:
PyObject *abstract_generator_next(PyObject *self, PyObject *args)
{
struct abstract_generator *generator = (struct abstract_generator *) self;
PyObject *retval;
// check running
retval = generator->body(self, None, 0);
// unlock running
return retval;
}
But this way should be much better, don't know if that is possible btw.
struct closure {
struct abstract_generator generator;
/* closure stuff here */
...
} ;
I add MarkGenerator visitor, that sets is_generator flag on DefNode
and checks for returns/yields.
Then CreateClosureClasses will create correct closure class for generator.
--
vitja.
___
Cython-dev mailing list
Cython-dev@codespeak.net
http://codespeak.net/mailman/listinfo/cython-dev
