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

Are the thread API's more inefficient than they need to be? #514

Open
cheesycod opened this issue Jan 21, 2025 · 1 comment
Open

Are the thread API's more inefficient than they need to be? #514

cheesycod opened this issue Jan 21, 2025 · 1 comment

Comments

@cheesycod
Copy link

cheesycod commented Jan 21, 2025

The current mlua thread API seems to push the values into the main Lua stack first before using xmove to move the pushed values from main stack to the thread state.

Would it not be more efficient to modify IntoLua trait to accept another parameter of lua_State to directly copy the values to thread state directly which would vastly reduce the number of stack operations performed by mlua?

@cheesycod cheesycod changed the title Are the thread API's inefficient? Are the thread API's highly inefficient? Jan 21, 2025
@cheesycod cheesycod changed the title Are the thread API's highly inefficient? Are the thread API's more inefficient than they need to be? Jan 21, 2025
@khvzak
Copy link
Member

khvzak commented Jan 21, 2025

which would vastly reduce the number of stack operations performed by mlua?

It's a single xmove call which translates to

from->top.p -= n;
for (i = 0; i < n; i++) {
    setobjs2s(to, to->top.p, from->top.p + i);
    to->top.p++;
}

where setobjs2s copy bits from one TValue to another.

Unless you're passing a significant number of arguments to the threads unlikely you will notice any difference. It's relatively lightweight operation and takes only fraction of time compared to running the entire thread to completion.

Would it not be more efficient to modify IntoLua trait to accept another parameter of lua_State to directly copy the values to thread state directly

Generally I agree that it would be more efficient. But it's a relatively large code change for a small use case with few edge cases that should be taken care of. Would be good to get confidence of performance boost before making any changes.

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

No branches or pull requests

2 participants