Slow when no matching founded

[eclipse-diffmerge-bot]

When computing the diff using match elements a contains() is often done on the completedMatches of EMapping.
However when no matching is found proxy resolving is done and it reallllly slow down the process.

I don't see the point of the proxy resolving here so I propose to disable it.

🆔 ECLIPSE-435046 👷 mathieu.velten 📅 2014-05-16 🔎 0.3.0

[eclipse-diffmerge-bot]

mathieu.velten commented on 2014-05-16

associated gerrit :
https://git.eclipse.org/r/#/c/26717/

olivier.constant commented on 2014-05-20

Thanks for the feedback, but could you give a bit more information? I understand your patch but I don't understand the need or the purpose.

"When computing the diff using match elements a contains() is often done on the completedMatches of EMapping."

I can't see what part of the computation process you are referring to. I does not seem to be in DiffOperation. Where is that in the code?

"However when no matching is found proxy resolving is done and it reallllly slow down the process."

Proxy resolving of target/referenceCompletedMatches? These references are subsets of modifiableCompletedMatches. Could you provide a usage scenario where you see a slow down due to these references so I can better understand the issue?

Thanks!

mathieu.velten commented on 2014-05-20

I went into this some times ago so the report was slightly inaccurate : the problem appears on the merge phase, when there is a lot of diff to merge.

When completeMatching is called (from mergeAddition/mergeValueAddition) an add is done on the completedMatches which triggers a contains, here is the stacktrace I hit when doing some sampling :

EObjectResolvingEList(EcoreEList).resolveProxy(EObject) line: 206
EObjectResolvingEList(EcoreEList).contains(Object) line: 387
EObjectResolvingEList(AbstractEList).add(E) line: 297
UnidirectionalComparisonCopier.completeMatch(IMatch, IComparison) line: 109
BidirectionalComparisonCopier.completeMatch(IMapping, IMatch) line: 60
EMappingImpl.completeMatch(IMatch) line: 474
EElementPresenceImpl.mergeAddition() line: 182
EElementPresenceImpl(EElementRelativePresenceImpl).doMergeIn(Role) line: 208
EElementPresenceImpl(EMergeableDifferenceImpl).mergeTo(Role) line: 791
MergeOperation.runOnSet() line: 184
MergeOperation.run() line: 135

olivier.constant commented on 2014-05-28

(In reply to mathieu.velten from comment #3)
Mathieu,

I was on a professional trip last week and I am currently on holidays until June 2nd. In the meantime, I had to generate the IP log for the Luna release due to the associated release train deadline. So whatever the outcome of our discussion, I'm not sure I can include your contribution to the Luna release when I am back at work. What is the consequence of that to you?

Back to the discussion: I would like to understand how the EMapping.reference/targetCompletedMatches can contain proxies. Are you using the usual "Compare with->Each other as models" action or are you is a custom usage scenario? If so, could you tell me a bit more about it?

(Note that my responsiveness will improve after June 2nd)

mathieu.velten commented on 2014-05-28

Hi Olivier,

That's the point, it can never contain a proxy except by manually doing some fragmenting on the Diff model, but I don't see the point...

However resolveProxy is set to true so when a match is not found it iterates on all the matches (again) and try to resolve them before retrying a .equals on each of them.

My use case is in our product in code with confidential models but you should be able to reproduce this by merging 2 big models (200 000 elements in my case) with a lot of addition (the contains is triggered by completeMatch, which is only called on mergeAddition and mergeValueAddition of the corresponding diff elements).

Regarding Luna : it would be great to have this merged in time, however this is far from blocking us since we are already using a patched version on Kepler.
Regarding IP log I am a verified committer, perhaps it could ease the process.

olivier.constant commented on 2014-06-04

(In reply to mathieu.velten from comment #5)

Dammit you are right. OK, I fully understand the issue now.

Besides the IP issue, I need to integrate your patch "manually": since it is based on an older version, there are changes that need to be made by hand - related to the Scope hierarchy for example.

Probably nothing complex, but given the timeline I propose to integrate your patch in the Luna SR1 release, and of course in 'master' shortly after the Luna release.

mathieu.velten commented on 2014-06-04

Fine for me.

The patch is really simple, juste change the resolveProxy attribute on targetCompletedMatches & referenceCompletedMatches and regenerate : I just saw that my current gerrit patch is quite messed up because of different EOL...

olivier.constant commented on 2014-06-04

(In reply to mathieu.velten from comment #7)

I tend to be very conservative so close to the release date, even for simple changes... Still, after patching the latest version myself, all non-regression tests are (as expected) OK... No IP issue since I did the simple tweak myself...

You convinced me, I'm willing to push it into the Luna release.

olivier.constant commented on 2014-06-04

In commit 7c67323.
Currently building.

olivier.constant commented on 2014-08-19

Closing.