Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 356063

Summary:

[builder] DefaultResourceDescriptionDelta requires same user data order in equals()

Product:

[Modeling] TMF

Reporter:

Knut Wannheden <knut.wannheden>

Component:

Xtext

Assignee:

Project Inbox <tmf.xtext-inbox>

Status:

CLOSED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

sebastian.zarnekow, sven.efftinge

Version:

2.0.1

Flags:

sebastian.zarnekow: indigo+

Target Milestone:

SR2

Hardware:

OS:

Mac OS X - Carbon (unsup.)

Whiteboard:

Attachments:

Description	Flags
patch for DefaultResourceDescriptionDelta	sven.efftinge: iplog+

Description Knut Wannheden

2011-08-29 06:37:27 EDT

The DefaultResourceDescriptionDelta will return true for haveEObjectDescriptionsChanged() if any of the compared EObject descriptions don't have the same order of the user data keys (see #equals(IEObjectDescription, IEObjectDescription)).

I think it is OK to expect this (but should be documented), but we must make sure all implementations also satisfy this requirement. This is not the case with descriptions returned by StatefulResourceDescription#copyExportedObjects() or CopiedResourceDescription(IResourceDescription).

The alternative is of course to change the comparison logic in DefaultResourceDescriptionDelta#equals().

Any thoughts on this?

Comment 1 Sebastian Zarnekow

2011-09-13 12:08:38 EDT

Knut, could you please provide a patch for this issue?

Comment 2 Knut Wannheden

2011-09-13 12:39:50 EDT

Hi Sebastian,

The question for me is how to fix the problem. From a performance perspective I would say that the IEObjectDescription#getUserDataKeys() array should be ordered and that DefaultResourceDescriptionDelta#haveEObjectDescriptionsChanged() should return "true" if this order is different. As a consequence all implementation classes should be updated. E.g. use ImmutableMap#copyOf() or similar when copying an IEObjectDescription.

Do you agree with the proposed solution?

Comment 3 Sebastian Zarnekow

2011-09-13 13:29:57 EDT

Since the constructor of org.eclipse.xtext.resource.EObjectDescription takes a map as a parameter. Do you really expect a significant performance penalty if you check them for equality in an order-independent manner? How many entries do you usually have in your user data?

Comment 4 Knut Wannheden

2011-09-13 15:42:37 EDT

Created attachment 203293 [details]
patch for DefaultResourceDescriptionDelta

We typically only have between 1 and 5 entries. But note that DefaultResourceDescriptionDelta also assumes that the getExportedObjects() maintains the insertion order for two IResourceDescriptions to be the same.

The problem can be addressed locally in DefaultResourceDescriptionDelta.java as in the attached patch. I could of course also write some test cases.

Comment 5 Sven Efftinge

2011-09-19 04:48:42 EDT

please apply. A test verifying the equals method works as expected would be good, too.

Comment 6 Knut Wannheden

2011-09-19 09:50:07 EDT

Pushed fix to master.

Comment 7 Karsten Thoms

2017-09-19 17:15:54 EDT

Closing all bugs that were set to RESOLVED before Neon.0

Comment 8 Karsten Thoms

2017-09-19 17:27:19 EDT

Closing all bugs that were set to RESOLVED before Neon.0