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

Improvement: Use EnumMap and fields for config #2033

Merged
merged 2 commits into from
Jun 12, 2024

Conversation

nea89o
Copy link
Contributor

@nea89o nea89o commented Jun 8, 2024

PR Reviews

When your PR is marked as ready for review, some of our maintainers will look through your code to make sure everything is good to go. In order to do this, they may request some changes you will need to do, or fix smaller stuff (like merge conflicts) for you. If a maintainer has reviewed your PR, make sure to pull any of their changes into your local project before doing more work on your code. Having maintainers fix small stuff for you helps us speed up the process of merging your PR, so if some of your systems warrant further care, be sure to let us know (preferably with a code comment).

Make sure to only mark your PR as "Ready to review" when it is. If you still want to do major changes, you can keep a draft PR open until then.

What

The config is accessed quite often and using a hash map for each one of these accesses adds up over time. This isn't suuper bad, but it is bad enough to the point where it does show up in profiles occasionally and the fix is quite easy. Using an EnumMap instead of a regular map is already a bit of a performance boost, but since SkyhanniMod.feature is used very often, I also added a bit of kotlin reflection to automatically set a field whenever it is loaded. Thanks to kotlins property reflection syntax this is fairly easy to use and extend, as well as type checked during compilation.

Going further

Almost every feature right now accesses the config multiple times per tick, sometimes per frame. While those accesses are just a few chained GETFIELDs, this could probably be improved. Config changes are rare and config changes within the same tick are something that we should simply not consider. The infrastructure for this does not exist right now, but one could imagine a future where moulconfigs properties are used extensively to reduce each config access down to at most two GETFIELDs each method call. Is this our future or one of our many divergent timelines? Is this future desirable? Is it worth the effort? I don't have the answer. I just know that performance can be found even in the most unlikely of places if you just open your eyes.

Changelog Improvements

  • Improved overall performance slightly by about ~1%. - nea

Changelog Technical Details

  • Added mixin search scope. - nea
  • Changed .gitignore so that subdirectories of the .idea folder can be manually unignored. - nea

@hannibal002 hannibal002 added this to the Version 0.26 milestone Jun 8, 2024
@hannibal002 hannibal002 added the Soon This Pull Request will be merged within the next couple of betas label Jun 8, 2024
@hannibal002 hannibal002 self-requested a review June 12, 2024 05:25
Copy link
Owner

@hannibal002 hannibal002 left a comment

Choose a reason for hiding this comment

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

jani hat nix anderes erwartet

@hannibal002 hannibal002 merged commit 488c0e3 into hannibal002:beta Jun 12, 2024
3 checks passed
@github-actions github-actions bot removed the Soon This Pull Request will be merged within the next couple of betas label Jun 12, 2024
@hannibal002 hannibal002 changed the title Use EnumMap and fields for config Improvement: Use EnumMap and fields for config Jun 12, 2024
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