This repository was archived by the owner on Jan 29, 2020. It is now read-only.
reduce arguments by reference#8
Open
marc-mabe wants to merge 15 commits intozendframework:developfrom
marc-mabe:feature/reduceArgsByRef
Open
reduce arguments by reference#8marc-mabe wants to merge 15 commits intozendframework:developfrom marc-mabe:feature/reduceArgsByRef
marc-mabe wants to merge 15 commits intozendframework:developfrom
marc-mabe:feature/reduceArgsByRef
Conversation
Member
|
I'll suggest to open a issue for each idea so we could discuss over each one separately. |
Member
Author
|
@Maks3w I have changed the title and description on this PR. Will open new for the other ideas. |
Member
|
@marc-mabe Thanks. Tiny objectives make it more easy to be approved and merged. |
Member
There was a problem hiding this comment.
/cc @weierophinney Is this the kind of cosmetic changes you want?
Member
Author
|
related to #67 |
Closed
Member
|
This repository has been closed and moved to laminas/laminas-cache; a new issue has been opened at laminas/laminas-cache#14. |
Member
|
This repository has been moved to laminas/laminas-cache. If you feel that this patch is still relevant, please re-open against that repository, and reference this issue. To re-open, we suggest the following workflow:
|
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This PR reduces arguments by reference often used on internal methods.
It was previously done as performance improvement but in fact it decreases performance as PHP have to create a new zval and in some cases it needs to copy the value before PHP-7 as described here: http://nikic.github.io/2015/05/05/Internal-value-representation-in-PHP-7-part-1.html
Additionally arguments by reference make the code more error prone and hard to read.