Fix DomainMessage metadata bottleneck #69
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
|
I have no idea why the tests failed. The failure seems unrelated but it didn't print anything useful to figure out the problem. |
|
@enumag the tests fails because the copyright needs to be updated to 2018. We have a travis hook for checking this, so we don't forget it somewhere. Can you update the As about the PR itself: I think I'm good with the changes, your thoughts @codeliner? |
|
LGTM 👍 |
|
@enumag if you could update the copyright year to 2018, then I'll merge and release this. |
|
@prolic Done. The build is passing now. |
prolic
approved these changes
Mar 22, 2018
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.
It turns out that adding metadata to a command or query is a serious bottleneck in our application because of the
toArrayandfromArraytransformations used in those methods. In our case it's about 10% of the total time the server needs to respond to a http request. The reason why it is a bottleneck is because the messages contain Ramsey\Uuid objects which are slow to convert to string and back. So I need to prevent this from happening.I've fixed it by changing the fromArray / toArray logic to simple PHP clone. Now this of course can be considered a BC break since the clone is no longer deep after this. I'm not exactly sure if this is a problem (it's not for me) so you need to consider the possible impacts before merging this. I'm fine with changing the logic on my side in case you don't want this change. Anyway I think you should know about this bottleneck and maybe consider some other changes to solve it.