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

Update some code handling strings where performance issues occur #688

Merged
merged 4 commits into from
Sep 27, 2024

Conversation

BenCheung0422
Copy link
Member

This improves a little performance issue. Here, Double#parseDouble is used to validate whether the input is double. It can be problematic as it throws exceptions when the input is invalid while the input is invalid most of the time. It is known that creation of exceptions are expensive due to the creations of stack traces. This pull request changes it to regex matching, though I doubt there would be an entry of number with localization enabled.

1~5% performance improvement is already significant when CPU time can be reduced. This kind of use cases (using methods with exceptions) will be carefully assessed when it can affect performance, in the code optimization task in the near future.

  • When mainly browsing world:
    image
  • When browsing around menus and displays full of text:
    image
  • When testing Add inventory capacity counter #686 (mainly with inventory menus):
    image
  • When browsing around menus and displays full of text with this pull request:
    image

@BenCheung0422 BenCheung0422 changed the title Change parseDouble to regex in Localization Update some code handling strings where performance issues occur Jun 20, 2024
@BenCheung0422
Copy link
Member Author

BenCheung0422 commented Jun 20, 2024

Furthermore, I have found another method with some performance issue (tested by browsing text-based displays):
image
Fixed by:
image
Reference: https://stackoverflow.com/a/35242882

@Litorom Litorom requested review from Makkkkus and Litorom August 30, 2024 02:58
@Litorom Litorom merged commit b197319 into MinicraftPlus:main Sep 27, 2024
1 check passed
Makkkkus pushed a commit that referenced this pull request Dec 5, 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