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
