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

meta: Fix even fewer object creations #1165

Conversation

hannibal002
Copy link
Member

This PR depends on:

Avoiding creating new objects even more often.

@jani270 jani270 changed the title Fix even fewer object creations meta: Fix even fewer object creations May 11, 2024
Copy link
Member

@nea89o nea89o left a comment

Choose a reason for hiding this comment

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

I am going to be honest, a lot of these "performance improvements" as of lately seem to be very misguided. I urge you to think about your code instead of blindly "optimizing" whatever statistic your new fancy profiler shows you. A lot of your performance improvements either break features, are shoddily implemented (some more complicated stuff like bad memoizations, relatively expensive hash functions, insufficient cache eviction, but also including simple typos!!!), or even make performance worse. I don't think I want to list all the minor details out here, all three of your PRs would be quicker to be done completely over with, rather than fixing your PR up.

Comment on lines +58 to +60
String rawName = entity.getName();
IChatComponent name = Utils.getOrPut(nameCache, rawName, entity::getDisplayName);
if (!entity.hasCustomName()) return name;
Copy link
Member

Choose a reason for hiding this comment

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

Fairly sure this will just create more performance issues? It will increase RAM usage and it will decrease raw CPU performance.

Copy link
Member Author

Choose a reason for hiding this comment

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

fair point.
Should we rather avoid touching IChatComponent here then? as this still creates new objects on every call, filling up the gc quickly.

@hannibal002
Copy link
Member Author

all three of your PRs would be quicker to be done completely over with, rather than fixing your PR up.

then please do so, or suggest better solutions.

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.

None yet

3 participants