Другой метод сравнения нарушает свой общий контракт!

Меня просят исследовать ошибку в рамках моей стажировки. Кусок кода кидает

java.lang.IllegalArgumentException: метод сравнения нарушает свой общий контракт!

Пользовательский Comparator сравнивает два пользовательских класса, просматривая long переменных-членов указанного пользовательского класса:

return v1 > v2 ? -1 : v1 < v2 ? 1 : 0;

Метод equals для этого пользовательского класса просматривает переменную-член String этого пользовательского класса. Мы чертовски долго воспроизводим поведение. Моей рефлекторной реакцией было заменить оператор return в пользовательском Comparator на return v2.compareTo(v1);, но моя команда сомневается, что это решит проблему. Может ли кто-нибудь предложить какое-либо понимание?

Arrays.sort(anArray, new Comparator<ACustomClass>() {
  @Override
  public int compare(ACustomClass o1, ACustomClass o2) {
    long v1 = o1.getALong();
    long v2 = o2.getALong();
    return v1 > v2 ? -1 : v1 < v2 ? 1 : 0;
  }});

person Dan Forbes    schedule 07.08.2013    source источник
comment
Есть ли шанс, что вы можете опубликовать реальный код?   -  person arshajii    schedule 08.08.2013
comment
Вы говорите, что у вас возникли проблемы с воспроизведением этой проблемы, но вы также говорите, что знаете, какая часть кода вызывает исключение. Что он?   -  person Greg Hewgill    schedule 08.08.2013
comment
Не могу публиковать ничего больше, чем уже есть... думаю, это нарушит мое соглашение о неразглашении. Я знаю код, который выдает исключение, но я не знаю пути данных/выполнения, из-за которого код выдает исключение... следовательно, у меня возникают проблемы с его воспроизведением.   -  person Dan Forbes    schedule 08.08.2013
comment
Можете ли вы изолировать подозрительную часть в SSCCE [sscce.org/]?   -  person PM 77-1    schedule 08.08.2013
comment
В документации Comparator есть некоторые примечания о согласованности с равными. Я бы сказал, что если возможно, что компаратор возвращает 0, когда equals() возвращает false, или если компаратор возвращает ненулевое значение, когда equals() возвращает true, у вас могут возникнуть проблемы.   -  person ajb    schedule 08.08.2013
comment
Почему бы вам просто не использовать Long.compare() здесь?   -  person arshajii    schedule 08.08.2013
comment
В дополнение к моему предыдущему комментарию: я быстро просмотрел источники среды выполнения, и сообщение исходит либо от TimSort, либо от ComparableTimSort. Эти классы, кажется, не вызывают equals() напрямую, но они могут вызывать что-то еще, что использует equals(). Но мне потребовалось бы много работы, чтобы углубиться в это глубже.   -  person ajb    schedule 08.08.2013
comment
Да, ты прав @ajb. Мы провели тонну отладки, но так и не смогли найти этот код.   -  person Dan Forbes    schedule 08.08.2013
comment
@arshajii, это было изменение, которое я предложил/внедрил. Однако, как я уже сказал, моя команда скептически относится к решению этой проблемы.   -  person Dan Forbes    schedule 08.08.2013
comment
Я хотел бы увидеть трассировку стека. Насколько я вижу, Arrays.sort() не выдает это исключение.   -  person user207421    schedule 08.08.2013
comment
Arrays.sort(...) вызывает TimSort(...), который выдает это исключение.   -  person Dan Forbes    schedule 08.08.2013
comment
Я думаю, что лучший способ решить эту проблему - использовать отладчик.   -  person Dawood ibn Kareem    schedule 08.08.2013
comment
Мы потратили на это довольно много времени с отладчиком... много разных тестов. Не удалось воспроизвести поведение. То, что происходит в TimSort, не совсем просто...   -  person Dan Forbes    schedule 08.08.2013
comment
@DanForbes Нет в исходном коде, который у меня есть: JDK 6 и OpenJDK 6. Что у вас?   -  person user207421    schedule 08.08.2013
comment
Метод сравнения нарушает свой общий контракт! ошибка была введена в Java 7, в частности, в ее методе Arrays.sort(Object[]) (тот, который использует TimSort). Но это должно возникать только тогда, когда задействован естественный порядок (а не пользовательский Comparator).   -  person Marko Topolnik    schedule 08.08.2013
comment
Вот обходной путь: установите системное свойство java.util.Arrays.useLegacyMergeSort. Ссылка: Совместимость с Java 7; поиск по RFE: 6804124.   -  person Marko Topolnik    schedule 08.08.2013
comment
EJP, правильно - это проблема JDK 7. @MarkoTopolnik, я думаю, что обе версии TimSort могут выдать его исключение. Мы видели этот обходной путь, но пытаемся выяснить, почему это происходит.   -  person Dan Forbes    schedule 08.08.2013
comment
В этом случае ваш компаратор не несовместим с равными, не так ли? Каждый проверяет отдельное поле.   -  person Marko Topolnik    schedule 08.08.2013
comment
К вашему сведению, я внимательно изучил исходный код TimSort, ища любой вызов метода, который мог бы вызвать какой-то метод в другом классе, который, возможно, мог бы вызвать equals() (что было бы тонкой ошибкой, если бы это произошло). Но я ничего не заметил. Так что это действительно делает этот ход мыслей намного менее вероятным. Если это проблема параллелизма, может помочь предложенная мной ранее идея добавления члена insideSort в ACustomClass, но на этот раз с отображением сообщения во всех методах, которые изменяют член ALong, если внутриSort имеет значение true. Сделать внутреннюю сортировку volatile.   -  person ajb    schedule 08.08.2013
comment
возможный дубликат Когда TimSort жалуется на неисправный компаратор?   -  person Raedwald    schedule 25.07.2014


Ответы (2)


Я не вижу ничего явно плохого в представленном компараторе. (И я скептически отношусь к предлагаемым исправлениям: мне они «пахнут» программированием вуду.)

Но если атрибут aLong класса ACustomClass был изменяемым... и он изменился во время сортировки... тогда код сортировки мог подумать, что компаратор нарушает контракт.

Итак... проверьте, может ли это быть проблемой параллелизма, когда один поток изменяет объекты в массиве, который пытается отсортировать другой поток.


Мы потратили на это довольно много времени с отладчиком... много разных тестов. Не удалось воспроизвести поведение.

Я бы рассматривал это как свидетельство, указывающее на проблему параллелизма...

person Stephen C    schedule 08.08.2013
comment
Точный метод IllegalArgumentException: Comparison нарушает его общий контракт! возникает только в Java 7 TimSort при сортировке в соответствии с естественным порядком. Это конкретное сообщение никогда не должно появляться при использовании Comparator. - person Marko Topolnik; 08.08.2013
comment
Я не думаю, что это правда, @MarkoTopolnik. - person Dan Forbes; 08.08.2013
comment
@StephenC - спасибо за понимание. Мы провели несколько тестов, чтобы убедиться, что это действительно так, и похоже, что вы правы. - person Dan Forbes; 14.08.2013
comment
@MarkoTopolnik везде поднимет свою уродливую голову; это не должно. Функция сортировки не должна предполагать, что объекты неизменны. Тот факт, что эта ошибка вызывает sort( T[], Comparator‹T› ), является конструктивным недостатком JDK. - person bond; 20.03.2016
comment
@bond - Функция сортировки не должна предполагать, что объекты неизменяемы. - Должна! Математически невозможно отсортировать набор объектов, отношения порядка которых меняются во время их сортировки. Это НЕ недостаток дизайна в JDK. Это недостаток дизайна в приложении, которое пытается сделать это... невозможное. - person Stephen C; 20.03.2016
comment
@StephenC зависит от того, что вы сортируете и как он используется. Для точной сортировки вам нужны неизменяемые объекты; для достаточно хорошей сортировки можно использовать изменяемые объекты. Проверка контракта должна быть работой компаратора, а не алгоритма сортировки. Если я хочу написать сортировку с использованием изменяемых объектов, встроенный алгоритм сортировки не должен наказывать меня. В основном это возвращается к оптимистичному спору о параллелизме и блокировке параллелизма. - person bond; 20.03.2016
comment
Что ж, контракт Java API для методов sort не подходит для достаточно хорошей сортировки. Это для точной сортировки, и так было всегда. Если вы хотите/нужна достаточно хорошая сортировка, вам нужно реализовать свои собственные методы сортировки. - person Stephen C; 20.03.2016

Я думаю, ты лаешь не на то дерево. Этот фрагмент кода не вызывает это исключение. Я хотел бы увидеть трассировку стека, а также посмотреть на метод ACustomClass.equals(). Если он не проверяет результаты getAlong() на равенство, и ничего больше, он не согласен с этим Comparator,, и поэтому один из них неверен, если используется в контексте, где требуется согласованность с равными, например отсортированная коллекция, которая, скорее всего, является источником исключения.

Вы можете устранить скептицизм экспериментальным путем. Если они не могут придумать фактическую формальную причину, по которой это не сработает, вы, безусловно, имеете право попробовать.

person user207421    schedule 08.08.2013
comment
equals не обязательно должно совпадать с компаратором. Arrays.sort не использует equals. - person arshajii; 08.08.2013
comment
@arshajii Это не говорит об этом в Javadoc, но говорит, что следует проявлять осторожность при использовании компаратора, способного навязывать порядок, несовместимый с равными, для упорядочения отсортированного набора (или отсортированной карты). Если мы смотрим на исходный код, он также не выдает это исключение. - person user207421; 08.08.2013