-
-
Notifications
You must be signed in to change notification settings - Fork 824
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
Refactor PlatformManager for performance #2151
base: master
Are you sure you want to change the base?
Conversation
Two improvements sure to provide a nice speedup to WorldEdit: 1. desynchronize `<PlatformManager>.queryCapability`, which is used in some pretty tight loops that would feel the benefits of less `synchronized`. 2. remove the use of `EnumMap` from `PlatformManager.preferences`, which further optimizes `<PlatformManager>.queryCapability` because `EnumMap`s do incur some overhead, mostly from checking to make sure the `enum` you passed it is really of the same class as the `enum` of the `EnumMap`.
Refactors for performance that make the code substantially messier (the fact you didn't follow the style guide at all aside), without any proof of speed-up or evidence that it's necessary, won't be accepted. Provide a microbenchmark and CPU profile that show why these changes make sense to do if you want to PR something like this. I'm also not convinced it couldn't be done in a substantially cleaner manner, if it's true that this has a performance impact |
All refactoring except for the removal of one `synchronized` was redacted.
I have significantly simplified these changes to the bare minimum: the removal of just the
Across the runs above, we see 40%-79% better performance for
Many thanks for your time and attention to this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather refactor the whole class to stop using synchronized
at all, and instead take advantage of a ReadWriteLock
(probably ReentrantReadWriteLock
to start with). It'll increase performance everywhere and is not as potentially dangerous as unguarded reads from a thread-unsafe map. If you don't feel up to that, please close this PR and I will file an issue to do so at a later date.
Two improvements sure to provide a nice speedup to WorldEdit:
<PlatformManager>.queryCapability
, which is used in some pretty tight loops that would feel the benefits of lesssynchronized
.EnumMap
fromPlatformManager.preferences
, which further optimizes<PlatformManager>.queryCapability
becauseEnumMap
s do incur some overhead, mostly from checking to make sure theenum
you passed it is really of the same class as theenum
of theEnumMap
.