IsDirty performance hit since 5.4.0

[trivalik]

I faced a longer IsDirty run after upgrading.

NHibernate 3.3.1:

NHibernate 5.4.2:

The first question is, was there a bug in old implementation that now lead to this cascade?
Since the result of IsDirty is true in both cases, I can imagine that there is optimization potential.

The test case of IsDirty is there that we have a table which stores for every element a parent element (not lazy load). The children are as HasMany relation. We remove just near all elements, and this lead to this bad result.

[hazzik]

@trivalik can you share full screenshot for 3.3.1 and also expand all trees for CascadeToOne?

[trivalik]

For 3.3.1 is through there was none, it was:

For 5.4.2 ( I can imagine that CascadeAfterSafe the problem is like in #1413):

[bahusoid]

You said you have the same performance issue with 5.3. Did you check with 5.2? Can you share how profiling looks for 5.3

[trivalik]

Wow. I wasn't aware that 5.3.16 has also 7ms.

[bahusoid]

I wasn't aware that 5.3.16 has also 7ms.

Then it's likely #2752 to blame

[trivalik]

I can confirm that the over 1 second run for IsDirty starts with the changes in commit 89a79d2.

[hazzik]

I ended up the following, but not sure if this is actually the repro

@trivalik, I'm not sure either.

Can you please test if changing this code:

nhibernate-core/src/NHibernate/Event/Default/AbstractFlushingEventListener.cs

Lines 187 to 202 in 90d50c0

	protected virtual void PrepareEntityFlushes(IEventSource session)
	{
	log.Debug("processing flush-time cascades");
	ICollection list = IdentityMap.ConcurrentEntries(session.PersistenceContext.EntityEntries);
	//safe from concurrent modification because of how entryList() is implemented on IdentityMap
	foreach (DictionaryEntry me in list)
	{
	EntityEntry entry = (EntityEntry) me.Value;
	Status status = entry.Status;
	if (status == Status.Loaded || status == Status.Saving || status == Status.ReadOnly)
	{
	CascadeOnFlush(session, entry.Persister, me.Key, Anything);
	}
	}
	}

And moving Anything out of the foreach loop fixes anything? Like this:

protected virtual void PrepareEntityFlushes(IEventSource session)
{
	log.Debug("processing flush-time cascades");

	ICollection list = IdentityMap.ConcurrentEntries(session.PersistenceContext.EntityEntries);
	var context = Anything; // <<<< HERE
	//safe from concurrent modification because of how entryList() is implemented on IdentityMap
	foreach (DictionaryEntry me in list)
	{
		EntityEntry entry = (EntityEntry) me.Value;
		Status status = entry.Status;
		if (status == Status.Loaded || status == Status.Saving || status == Status.ReadOnly)
		{
			CascadeOnFlush(session, entry.Persister, me.Key, context /* <<<< HERE */);
		}
	}
}

[trivalik]

Moving out the Anything, out of the foreach loop lead to no SaveOrUpdate call. And solved it for my private test case and for the test case I change in NHibernate.

[hazzik]

@trivalik great. Can you submit a PR?

[hazzik]

loop lead to no SaveOrUpdate call.

I cannot get the SaveOrUpdate call with your code.

[trivalik]

You are right the unit test does not create a call of FireSaveOrUpdate. But I verified now with 5.4.x branch (before only with the old commit when error comes in), that the calls for CascadeOn is increased. Do not look on the ms because this profiling was to calculate the exact call count, not the times.

Here for the 5.4.x branch:

Here with the Anything fix: