Fix #16411, added ORM SessionCache AKA FirstLevelCache#16450
Fix #16411, added ORM SessionCache AKA FirstLevelCache#16450rudiservo wants to merge 1 commit intophalcon:5.0.xfrom
Conversation
7e21423 to
ddbcbe2
Compare
|
SqLite tests fail, the tests are fixed in one of the PR I previously made, I will rebase this once they are approved. |
|
@rudiservo I had some time (finally) to check this one. I see the benefit and am not opposed to it but there are some concerns I have in particular with the __destruct part and orphan references. We can certainly add checks for objects that are null (modelsManager etc.) and that is fine but I am worried about the internal dependencies of objects and the references that they hold to other objects. Therefore the Finally, do we really need a reference to the modelsManager? Can't we use the cache to store all that and only keep the key? |
|
Hey, the __destruct will be called by the php GC, meaning that the worst case scenario is a GC bug and there is no longer a single source of truth, that is what we have now with the current system, this is only to remove keys that no longer has any stable reference. So the philosophy of a first level cache system is a single source of truth, meaning that anywhere on the current thread the Model called is always the same. I think we could pass a cache, in fact, if we are using something like Swoole or Kraken we should have a new cache for each thread, because every thread is independent on its working data. I'll check it out. |
Hello!
In raising this pull request, I confirm the following:
Small description of change:
Thanks