Replies: 5 comments 1 reply
-
@gofal do you have an opinion on this as we currently are not updating to the latest version due to the number of breaking changes it has introduced (and the care needed now when using a link statement) |
Beta Was this translation helpful? Give feedback.
-
I try to be cautous not to intoduce breaking changes or risks without explicitly announcing it. I did some benchmarks: the calculation of the Hashcode should not be a problem, the code is super fast (so 10 to 15 Microseconds) and allocates no memory. |
Beta Was this translation helpful? Give feedback.
-
DicomDataset inherits from IEquatable, this is something you cannot change with a blobal option. But the DicomDatasetComparer, that is used under the hood, could have some global options like "Only compare with ReferenceEquals" or "Compare all content". Please open an issue where we can address this and add some global options, before we do the next release. |
Beta Was this translation helpful? Give feedback.
-
created issue #1807 |
Beta Was this translation helpful? Give feedback.
-
Hi,
I am unable to update FO-dicom 5.1.2. since a recent update which add a comparer for datasets
I assume it is from "Added IEquatable implementation and equality operators for DicomDataset class"
This seems to affect us where previously references were used in explicit dictionaries and implicit hashsets in LINQ queries.
I am assuming that when comparing is going on in these collections a huge object is created to keep the item used to compare - and the object may stay around for the life of the dataset ?
This explodes our memory use to 10s of GB for our process (leading to out of memory exceptions ) compared to 100s of MB and it much slower when it does not run out of memory.
I assume we can make our code compliant by using our own comparer, but care is need to spot where LINQ is creating hash sets.
I would have expected this to be listed under the breaking changes section (which it is not) and even a more major version increase due to impact.
Others may be affected in a similar way, but it could be more subtle for them.
Would a global option to enable the previous behaviour a good migration path ?
Thoughts ?
Tony
Beta Was this translation helpful? Give feedback.
All reactions