Am Donnerstag, 29. März 2007 00:00 schrieb Alek Storm: > On 3/28/07, Leopold Toetsch <[EMAIL PROTECTED]> wrote: > > 1) This ism't needed, these pointers are only valid and needed up to the > > next > > sub call/return. > > The current_results member already lives in Parrot_Context; this patch > just moves the rest of them there as well. Variables can't be global > simply because of when they're valid. It's a hack.
current_results is valid until the sub returns. Therefore it's in Context. This isn't a reason for putting similar variables into the same place, which are just valid for a range of 3 or some such opcodes. > > 2) The Context struct should be as small as possible because each sub > > call needs one. > > Compactness does not supersede good design. Basically & in theorie yes, but don't always forget performance. Any interpreter built on good & clean design principles based on e.g. SKI combinator calculus will suck speed-wise. More towards reality ;) we also have to take compromises in the implementation, always. (And please see below) > > 3) Reentrancy isn't a problem as new threads get new interpreter structs > > anyway. > > Reentrancy isn't only a threading issue. From src/inter_run.c, in > Parrot_runops_fromc_args_event (line 318): > /* running code from event handlers isn't fully reentrant due to > * these interpreter variables - mainly related to calls */ > The code then has to manually save these variables, then restore them > after the PIR sub is called. Thus, reentrancy *is* a problem. With > this patch, the number of variables we have to do this with is reduced > to one. First [0]. But indeed this is a bug in the event code (which isn't really specced and just there for experimenting with it). The Plan(tm) is to switch to other event's code at "safe points". This is for sure not inside C code, but also not inside a function call sequence, where these pointers are used currently. Such a call/return sequence shall be considered as one "basic operation", where no code switch might occur. The current implementation is buggy WRT that. Depending on the actual runloop we have possible code switches to event handlers at each opcode down to practically no [1] event handling at all inside JIT-only code. I've long ago mentioned that we should mark opcodes [2] that might switch to a different (event handler) runloop with some metasyntax in the .ops files. There's already a macro that does the necessary flag checking and event loop handling if needed. The "good design" here (IMHO) is to fix the event code and not to try to adjust other parts of the interpreter because of the mentioned reasons. > Thanks, > Alek leo [0] Well, you would have to prove that there are typicall a lot of event code switches and that the related save/resore sequence of current_ ptrs does hurt more than joe average sub call performance penalties your patch imposes, but lets forget that for a while ;) [1] except the sleep opcode. [2] backward loop ops, whatever longer taking opcode ... (too be decided)
