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.

Sign up for GitHub

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

Jump to bottom

Rewrite E2 table clone function/partial table extension modernization #3195

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Rewrite E2 table clone function/partial table extension modernization #3195

wants to merge 6 commits into from
+334 −130

Conversation

Denneisk
Copy link
Member

@Denneisk Denneisk commented Dec 2, 2024

  • Rewrote the implementation of table:clone().
    • It's faster. CPU time down 30-50%, even without any recursion.
    • Fixes Table operations break E2 lambdas #3193.
    • For better or worse, Vector2s, Vector4s, Matrices, Quaternions, and all the other number-struct types are now copied by reference always. Same applies for object-like tables, but that's assuredly a good thing.
    • Op costs are reduced roughly 10-fold (the old function made no sense on how it applies ops) from my testing
  • Added table:copy() to make a shallow copy of a table.
    • Name can be changed if you prefer :P
  • Added some helper functions in the table extension file, notably for performance limiting
    • The design goal was to halt the chip before table operations are done, instead of after like they do currently.
  • Modified some functions to be more efficient.

@Denneisk
Rewrite table copy functions
e6f5092 
Add shallow table copying
Add table perf function
Minor optimizations and cleanups
@Denneisk
E2descriptions
24bc3ad
@Denneisk
Remove references to arrayCopy
75c57cc 
It was a stub, NOT AI-generated. I swear.
@Denneisk
Combine these loops
c78e69f
@Denneisk
Remove unnecessary return statement
0bd71b5
@Denneisk
Copy link
Member Author

Denneisk commented Dec 2, 2024 •
edited
Loading

Added test case that covers every significant modification. Note that it won't pass until #3197 is merged (commenting out the offending line will resolve this as a temporary fix).

@thegrb93
Copy link
Contributor

thegrb93 commented Dec 2, 2024

You could do this instead of goto

while true do
    -- copy stuff

    if not n then break end
    -- update stuff
end

@Denneisk
Copy link
Member Author

Denneisk commented Dec 2, 2024 •
edited
Loading

With such a small loop, the performance difference is negligible, but I'm basing this off a benchmark that suggests goto would be one of the fastest options. It's not a perfect comparison but I think it's reasonable to extrapolate from.

I don't know, it doesn't really matter that much probably. I just thought it was cool...

A while loop is 2 IL instructions longer. Think of the cache misses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thegrb93 thegrb93 thegrb93 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Table operations break E2 lambdas
2 participants
@Denneisk @thegrb93