Fix(Hydrator): disabling identity map now disables the identity map#131
Fix(Hydrator): disabling identity map now disables the identity map#131vgreb merged 2 commits intoccmbenchmark:masterfrom
Conversation
|
Wow ! Good Catch ! Does this bug exists since IdentityMap was introduced ? |
|
maybe I should give second thoughts on this one. This change seems really needed but I should probably check there is no unintended side effects more carefully. Seems to be there for a while now, really don't think it's on purpose but these lines makes me doubt: https://github.com/ccmbenchmark/ting/pull/131/changes#diff-1a1e1cbffbe64904034ef60574a22c8b697439bdb644371209c4c7c2a21c5093L337-L339 if (isset($this->references[$ref]) === true) {
// This entity was already created and stored into references
if ($this->identityMap === false) {
// If identityMap is disabled, it clones the object and reset UUID
$result[$column['table']] = clone $this->references[$ref];
} else {
// (...)
}
// (...)
} |
dbb1721 to
2bccf3e
Compare
|
⏺ Here is the full story Comparison:
What was happening Even with identityMap disabled:
So the behavior appeared correct on the surface (objects were cloned), but memory was still being consumed by storing all references. This fix now prevents storage when identityMap is disabled. |
vgreb
left a comment
There was a problem hiding this comment.
Got it ! Thank you for this fix.
Noticed today with a big batch of reads, when trying to optimize the memory consumption, that the identity map was not properly disabled. This PR fixes that.