perf: replace isValueType with faster alternative #1617
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Kudos, SonarCloud Quality Gate passed!
|
|
@paulpach why do we still need the check? |
|
All our messages are value type, but there is no guarantee/requirement that messages are value types. In fact our documentation teaches people to use classes because they happen to be more convenient. With classes you don't have to add an empty Serialize and Deserialize method. This PR should fix the problem though |
|
did you profile it already? |
According to vis benchmark here #1614 (comment) isValueType is an expensive operation. This microoptimization replaces isValueType for a faster (not so readable) alternative
|
profiling, hang in there... |
|
Before: After: |
github-actions bot
referenced
this pull request
in MirageNet/Mirage
Mar 28, 2020
## [29.0.3](29.0.2-master...29.0.3-master) (2020-03-28) ### Performance Improvements * faster NetworkWriter pooling ([#1616](https://github.com/MirrorNG/MirrorNG/issues/1616)) ([5128b12](5128b12)), closes [#1614](https://github.com/MirrorNG/MirrorNG/issues/1614) * replace isValueType with faster alternative ([#1617](https://github.com/MirrorNG/MirrorNG/issues/1617)) ([61163ca](61163ca)), closes [/github.com/MirrorNetworking/Mirror/issues/1614#issuecomment-605443808](https://github.com//github.com/vis2k/Mirror/issues/1614/issues/issuecomment-605443808) * use byte[] directly instead of MemoryStream ([#1618](https://github.com/MirrorNG/MirrorNG/issues/1618)) ([166b8c9](166b8c9))
github-actions bot
referenced
this pull request
in MirageNet/Mirage
Mar 28, 2020
According to vis benchmark here MirrorNetworking/Mirror#1614 (comment) isValueType is an expensive operation. This microoptimization replaces isValueType for a faster (not so readable) alternative
github-actions bot
referenced
this pull request
in MirageNet/Mirage
Mar 28, 2020
## [29.0.3](29.0.2-master...29.0.3-master) (2020-03-28) ### Performance Improvements * faster NetworkWriter pooling ([#1616](https://github.com/MirrorNG/MirrorNG/issues/1616)) ([5128b12](5128b12)), closes [#1614](https://github.com/MirrorNG/MirrorNG/issues/1614) * replace isValueType with faster alternative ([#1617](https://github.com/MirrorNG/MirrorNG/issues/1617)) ([61163ca](61163ca)), closes [/github.com/MirrorNetworking/Mirror/issues/1614#issuecomment-605443808](https://github.com//github.com/vis2k/Mirror/issues/1614/issues/issuecomment-605443808) * use byte[] directly instead of MemoryStream ([#1618](https://github.com/MirrorNG/MirrorNG/issues/1618)) ([166b8c9](166b8c9))
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
According to vis benchmark in #1614
isValueType is an expensive operation.
This micro-optimization replaces isValueType for a faster (not so readable) alternative