Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Idempotent optimisation of bytecode lookup. #104

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vext01
Copy link
Contributor

@vext01 vext01 commented Mar 6, 2025

Requires ykjit/yk#1642

By promoting the bytecode address and moving the load out of the bytecode array into an idempotent function we get a decent speedup of around 32% on bigloop.

Is this safe?

ltratt:

all we have to worry about is whether a function's bytecodes can
change without us telling yk. there are two ways this could happen:

a. (most obvious) someone literally mutates the bytecode with
something like f->code[2] = OP_...

b. (less obvious) if yklua changes a function's bytecode allocation.
notably, if it does free(f->code) then f->code = malloc(...)
and got back the same pointer, then this would be a mutation from our
perspective

Edd investigates and comes back with:

I don't see any meaningful "mutations" of the bytecode array that would
matter to us. The array grows as the parser is building the program, it
mutates it locally to do peephole optimisations immediately after the parser
is done, then it shrinks the array in case the peephole optimiser eliminated
instructions. Then it's freed when the function proto dies.

All of that to say, we believe this is a safe optimisation.

By promoting the bytecode address and moving the load out of the
bytecode array into an idempotent function we get a decent speedup of
around 32% on bigloop.

Is this safe?

ltratt:
> all we have to worry about is whether a function's bytecodes can
> change without us telling yk. there are two ways this could happen:
>
> a. (most obvious) someone literally mutates the bytecode with
> something like `f->code[2] = OP_...`
>
> b. (less obvious) if yklua changes a function's bytecode allocation.
> notably, if it does `free(f->code)` then `f->code = malloc(...)`
> and got back the same pointer, then this would be a mutation from our
> perspective

Edd investigates and comes back with:
> I don't see any meaningful "mutations" of the bytecode array that would
> matter to us. The array grows as the parser is building the program, it
> mutates it locally to do peephole optimisations immediately after the parser
> is done, then it shrinks the array in case the peephole optimiser eliminated
> instructions. Then it's freed when the function proto dies.

All of that to say, we believe this is a safe optimisation.
@@ -1169,12 +1169,23 @@ void luaV_finishOp (lua_State *L) {

/* fetch an instruction and prepare its execution */
#ifdef USE_YK
// Elide instruction lookup.
//
// FIXME: return type should be `Instruction` not `uint64_t`. We only do this
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix this in a follow up PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants