2010/12/6 Stefan Behnel <[email protected]>:
> 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;
>> // 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?)
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.
>
>> 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...
>
Now I check for yield/returns inside MarkGenerator, and set
is_generator flag or raise error if both yield and return was found.
Guess that's not right place for this.
--
vitja.
_______________________________________________
Cython-dev mailing list
[email protected]
http://codespeak.net/mailman/listinfo/cython-dev
