From: Bob Rogers <[EMAIL PROTECTED]> Date: Mon, 21 Apr 2008 18:54:19 -0400
From: "Patrick R. Michaud" <[EMAIL PROTECTED]> Date: Mon, 21 Apr 2008 15:42:27 -0500 . . . If it looks like having a separate stack for bsr/ret is workable then I'll go at it that way. Fair enough. FWIW, I'm now working on a simple hack that should show how much speedup we can expect. The hack is attached; it isn't headerized, has some coding standards failures, and leaks memory, so it's incomplete. Disappointingly, it produces an overall speedup of less than 4% in building gen_actions.pir in r27087 (which has chromatic's ref-counting speedup). And it fails t/pmc/continuation.t test 4 ("continuations preserve bsr/ret state"), as expected. I don't think a 4% speedup is worth it, so I'm giving up on it. Maybe somebody else will find a way to improve it. A warning, though: Do "make clean" after applying the patch, or the new "parrot" will consume all memory. I suspect there is some code that uses Parrot_Context that doesn't have the right dependencies, so is not recompiled when interpreter.h is changed. -- Bob [P.S. Oops.]
*** Give bsr/ret a stack of their own *** * include/parrot/interpreter.h: + Add bsr_stack, bsr_allocated, and bsr_stack_size members to Parrot_Context. * src/gc/register.c: + (init_context): Initialize bsr_stack. * src/stacks.c: + (Parrot_bsr_stack_push, Parrot_bsr_stack_pop): New. (Probably need a better permanent home for them.) * src/ops/core.ops: + (bsr, ret, jsr): Use Parrot_bsr_stack_push/Parrot_bsr_stack_pop. Diffs between last version checked in and current workfile(s): Index: include/parrot/interpreter.h =================================================================== --- include/parrot/interpreter.h (revision 27087) +++ include/parrot/interpreter.h (working copy) @@ -218,6 +218,10 @@ struct Stack_Chunk *user_stack; /* Base of the scratch stack */ PMC *lex_pad; /* LexPad PMC */ struct Parrot_Context *outer_ctx; /* outer context, if a closure */ + opcode_t **bsr_stack; /* pointer to bsr/ret addresses, lazily allocated + by Parrot_bsr_stack_push. */ + UINTVAL bsr_allocated; /* allocated bsr_stack size */ + UINTVAL bsr_stack_size; /* used bsr_stack size */ UINTVAL warns; /* Keeps track of what warnings * have been activated */ UINTVAL errors; /* fatals that can be turned off */ Index: src/gc/register.c =================================================================== --- src/gc/register.c (revision 27087) +++ src/gc/register.c (working copy) @@ -297,6 +297,9 @@ ctx->current_cont = NULL; ctx->current_object = NULL; /* RT#46181 who clears it? */ ctx->current_HLL = 0; + /* We don't need to initialize bsr_allocated or bsr_stack_size, because they + won't be examined if bsr_stack is NULL. */ + ctx->bsr_stack = NULL; if (old) { /* some items should better be COW copied */ Index: src/stacks.c =================================================================== --- src/stacks.c (revision 27087) +++ src/stacks.c (working copy) @@ -484,6 +484,49 @@ height, dynamic_env, dynamic_env->name); } +/*** bsr/ret stack support ***/ + +PARROT_API +void +Parrot_bsr_stack_push(PARROT_INTERP, ARGIN(opcode_t *return_addr)) +{ + parrot_context_t *context = CONTEXT(interp); + + if (! context->bsr_stack) { + /* Need a new stack. */ + UINTVAL size = 100; + /* fprintf(stderr, "[first alloc for %p]\n", context); */ + context->bsr_allocated = size; + context->bsr_stack_size = 0; + context->bsr_stack = (opcode_t **)mem_sys_allocate(size*sizeof(opcode_t *)); + } + else if (context->bsr_stack_size >= context->bsr_allocated) { + /* Need a bigger stack. */ + UINTVAL size = 2*context->bsr_allocated; + /* fprintf(stderr, "[realloc for %p (currently %p), from %d to %d]\n", + context, context->bsr_stack, + (int)context->bsr_allocated, (int)size); */ + if (size > 1000000) + real_exception(interp, NULL, 1, "Too many 'bsr' calls\n"); + context->bsr_stack = (opcode_t **)mem_sys_realloc(context->bsr_stack, size*sizeof(opcode_t *)); + context->bsr_allocated = size; + } + + /* Push it. */ + context->bsr_stack[context->bsr_stack_size++] = return_addr; +} + +PARROT_API +opcode_t * +Parrot_bsr_stack_pop(PARROT_INTERP) +{ + parrot_context_t *context = CONTEXT(interp); + + if (! context->bsr_stack || context->bsr_stack_size == 0) + real_exception(interp, NULL, 1, "'ret' without 'bsr'\n"); + return context->bsr_stack[--context->bsr_stack_size]; +} + /* =back Index: src/ops/core.ops =================================================================== --- src/ops/core.ops (revision 27087) +++ src/ops/core.ops (working copy) @@ -101,6 +101,8 @@ =cut +/* ' */ + inline op noop() :base_core { goto NEXT(); } @@ -217,8 +219,7 @@ =cut inline op bsr(label INT) :base_core,check_event { - stack_push(interp, &interp->dynamic_env, - expr NEXT(), STACK_ENTRY_DESTINATION, STACK_CLEANUP_NULL); + Parrot_bsr_stack_push(interp, expr NEXT()); goto OFFSET($1); } @@ -229,7 +230,8 @@ =cut inline op ret() { - goto POP(); + opcode_t * loc = Parrot_bsr_stack_pop(interp); + goto ADDRESS(loc); } @@ -244,8 +246,7 @@ inline op jsr(label INT) :base_core,check_event { opcode_t * loc; - stack_push(interp, &interp->dynamic_env, - expr NEXT(), STACK_ENTRY_DESTINATION, STACK_CLEANUP_NULL); + Parrot_bsr_stack_push(interp, expr NEXT()); loc = INTVAL2PTR(opcode_t *, $1); goto ADDRESS(loc); } End of diffs.