Change tracking policy "NOTIFY" seems to be useless #5610

New Issue

admin · 2026-01-22T15:12:38+01:00

admin commented

2026-01-22 15:12:38 +01:00

Originally created by @eschultz-magix on GitHub (Jul 19, 2017).

Sorry for the aggressive title, but I am fighting with the notify change tracking policy as described here: http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/reference/change-tracking-policies.html

That way you also have full control over when you consider a property changed. [...] The positive point and main advantage of this policy is its effectiveness. It has the best performance characteristics [...] and a flush() operation is very cheap when nothing has changed.

This seems to be false. Calling flush() will call UnitOfWork::commitChangeSets() and it will only proceed entities that have been scheduled for synchronization which is done by calling the propertyChanged event. So far, so good.

But then for each managed entity that has been updated UnitOfWork::computeChangeSet() is called which will always walk through the reflection fields, save the actual data and compare it with the original data. The difference between the "deferred implicit" and the "notify" change tracking policy is that "notify" will start with the prepared changeset whereas "deferred implcit" uses an empty changeset, but both will override if the UnitOfWork itself detects a change between the original data and the reflected actual data. So both are performing a dirty check.

TL;DR: "NOTIFY" should rely on the given changeset without reflecting fields and comparing data.

Originally created by @eschultz-magix on GitHub (Jul 19, 2017). Sorry for the aggressive title, but I am fighting with the notify change tracking policy as described here: http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/reference/change-tracking-policies.html ```That way you also have full control over when you consider a property changed. [...] The positive point and main advantage of this policy is its effectiveness. It has the best performance characteristics [...] and a flush() operation is very cheap when nothing has changed.``` This seems to be false. Calling ```flush()``` will call ```UnitOfWork::commitChangeSets()``` and it will only proceed entities that have been scheduled for synchronization which is done by calling the ```propertyChanged``` event. So far, so good. But then for each managed entity that has been updated ```UnitOfWork::computeChangeSet()``` is called which will **always** walk through the reflection fields, save the actual data and compare it with the original data. The difference between the "deferred implicit" and the "notify" change tracking policy is that "notify" will start with the prepared changeset whereas "deferred implcit" uses an empty changeset, **but both will override if the ```UnitOfWork``` itself detects a change between the original data and the reflected actual data**. So both are performing a dirty check. **TL;DR:** "NOTIFY" should rely on the given changeset without reflecting fields and comparing data.

admin added the Bug Missing Tests labels 2026-01-22 15:12:38 +01:00

admin commented

2026-01-22 15:12:39 +01:00

@Ocramius commented on GitHub (Jul 21, 2017):

The difference is that diffing is completely skipped if the change is not notified, saving huge amounts in CPU cycles for entities that didn't change at all.

That's pretty much the reason for the "notify" naming

@Ocramius commented on GitHub (Jul 21, 2017): The difference is that diffing is completely skipped if the change is not notified, saving huge amounts in CPU cycles for entities that didn't change at all. That's pretty much the reason for the "notify" naming

admin commented

2026-01-22 15:12:41 +01:00

@eschultz-magix commented on GitHub (Jul 24, 2017):

@Ocramius But why should I notify about a property change including the changeset if the changeset is built up again?

I really like the approach to notify about the exact changes, but that changeset should be used as-is without reflecting the fields again. This will save a lot more CPU cycles. Maybe you can add this as new policy?

@eschultz-magix commented on GitHub (Jul 24, 2017): @Ocramius But why should I notify about a property change including the changeset if the changeset is built up again? I really like the approach to notify about the exact changes, but that changeset should be used as-is without reflecting the fields again. This will save a lot more CPU cycles. Maybe you can add this as new policy?

admin commented

2026-01-22 15:12:41 +01:00

@Ocramius commented on GitHub (Jul 24, 2017):

@eschultz-magix I need to check, but I suggest writing a test that works against past versions too: I'm fairly sure it used to work like you describe it.

@Ocramius commented on GitHub (Jul 24, 2017): @eschultz-magix I need to check, but I suggest writing a test that works against past versions too: I'm fairly sure it used to work like you describe it.

admin commented

2026-01-22 15:12:41 +01:00

@beberlei commented on GitHub (Dec 6, 2020):

It only worked like this at some point during alpha/beta, then we realized that its not really that ismple and requires users to understood much of the internals, so we had to change it. Indeed notify is very similar to deferred explicit.

It should be deprecated in favor of deferred explicit in the future.

@beberlei commented on GitHub (Dec 6, 2020): It only worked like this at some point during alpha/beta, then we realized that its not really that ismple and requires users to understood much of the internals, so we had to change it. Indeed notify is very similar to deferred explicit. It should be deprecated in favor of deferred explicit in the future.

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: doctrine/archived-orm#5610