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

Implement --@performant tag #1920

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RaphaelIT7
Copy link

What is this

The --@performant is a tag that can be used on functions that do a lot of logic/math to improve their performance by removing the debug hook(cpu checks) before their execution and adding it back afterward.

Now, if there wouldn't be any restrictions, this could easily be abused.
So to for the --@performant tag to take action, these conditions must be met.
- The function can't use for, while, repeat and goto
- The function can't call another function
- The function can't have any child functions
(Currently limited because else my code would die/the mapping I'm doing to verify functions would end up being incorrect)
- The function must be global/on the _G table.
(Currently limited to this since I can't think of a good way to get a local function without it being a bad workaround)

The goal is it to end up allowing small functions to run without the cpu checks which greatly speeds up their execution time, while also trying to make sure that they can't freeze the game.

Example

without and with it.

performant_tag.mp4

The Script used:
starfall_particles.txt

Changes

- Modified SF.CompileString to have a second return value
(a table that contains information about the compiled functions, similar to debug.getinfo)
- Added SF.Instance:readdCpuCheck()
Restores the debug hook in the case it was removed.
- Added SF.Instance.CompileHighPerformance
Implements the --@performant tag.
- Modified SF.Instance.Compile to call SF.Instance.CompileHighPerformance

ToDo

Things I have to do, before this is ready.

  • Let ValidateHighPerformanceFunction account for comments.
  • Go through all the code again and check the codestyle
  • Somehow account for metatables since those could possibly make function calls?
  • do a lot more testing

@Vurv78
Copy link
Contributor

Vurv78 commented Dec 13, 2024

No

@thegrb93
Copy link
Owner

I don't think the speedup will be worth all the possible disasterous bugs and consequences this will bring. You'll have better luck adding a cvar to increase the debug.sethook counter size.

@RaphaelIT7
Copy link
Author

RaphaelIT7 commented Dec 13, 2024

From what I found, having the hook alone is causing issues.
The counter size doesn't really seem to matter, as long as the hook exists the performance will be worse.

Example (Tested on Windows):
lua_run local a = SysTime() debug.sethook(function() end, "", 10000000000000000000000000000) for k=1, 10000000 do end debug.sethook() print(SysTime() - a)
-> 0.9688519

lua_run local a = SysTime() debug.sethook(function() end, "", 100) for k=1, 10000000 do end debug.sethook() print(SysTime() - a)
-> 0.9761029

@Vurv78
Copy link
Contributor

Vurv78 commented Dec 13, 2024

From what I found, having the hook alone is causing issues. The counter size doesn't really seem to matter, as long as the hook exists the performance will be worse.

Example (Tested on Windows): lua_run local a = SysTime() debug.sethook(function() end, "", 10000000000000000000000000000) for k=1, 10000000 do end debug.sethook() print(SysTime() - a) -> 0.9688519

lua_run local a = SysTime() debug.sethook(function() end, "", 100) for k=1, 10000000 do end debug.sethook() print(SysTime() - a) -> 0.9761029

Well yes, afaik setting a hook on a function will prevent jitting.
But we don't really want a function to jit cause then there's nothing you can do to prevent using a ton of resources.

@yogwoggf
Copy link

The speedup is also very small

@wrefgtzweve
Copy link
Contributor

A 4x performance boost would be pretty nice to optionally have

@thegrb93
Copy link
Owner

superuser disables the cpu checks, and you can also disable clientside checks for your own chips.

@Vurv78
Copy link
Contributor

Vurv78 commented Dec 14, 2024

Even if you wanted this, this is not the way to go about it. It has a lot of gaps to exploit and absolutely shouldn't be parsing anything. You could implement this in like 1/10 the code and more securely by instead scanning the bytecode with string.dump..

Repository owner deleted a comment from yogwoggf Dec 14, 2024
@thegrb93
Copy link
Owner

@yogwoggf don't flame or shame

Repository owner deleted a comment from yogwoggf Dec 14, 2024
@yogwoggf
Copy link

@yogwoggf don't flame or shame

i did neither but ok 👍

@thegrb93
Copy link
Owner

You did both

@RaphaelIT7
Copy link
Author

Even if you wanted this, this is not the way to go about it. It has a lot of gaps to exploit and absolutely shouldn't be parsing anything. You could implement this in like 1/10 the code and more securely by instead scanning the bytecode with string.dump..

I know that this code is quite bad and could currently have gaps/exploits, since why this is still a draft.
I will probably change a few things if not the entire code later as for example I completely forgot that string.dump existed, so I'll look into possibly using that, thanks for the suggestion :D

I don't think the speedup will be worth all the possible disasterous bugs and consequences this will bring.

yes, this entire pr might end up as a failure if it proves to be impossible/too difficult or too unsafe, but I will at least try.

@Vurv78
Copy link
Contributor

Vurv78 commented Dec 14, 2024

Draft doesn't exempt a pr from getting any feedback at all, I didn't review anything. Just saved you a ton of time.

@Cheatoid
Copy link
Contributor

Cheatoid commented Jan 4, 2025

Good idea. Wrong approach. Also yes, setting up a metatable would escape your validation.

and you can also disable clientside checks for your own chips.

What if I want to disable clientside checks for my friend's SF chip? Also what's the command for that... maybe add this option when right-clicking a chip on context menu (C).

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.

6 participants