RFR: 8303526: Changing "arbitrary" Name.compareTo() ordering breaks the regression suite [v2]

Fri Mar 24 22:23:29 UTC 2023

On Fri, 24 Mar 2023 17:31:58 GMT, Vicente Romero <vromero at openjdk.org> wrote:

> sorry if I'm missing something but I'm not totally sure what we are fixing here, I mean it could be that the current code is not arbitrarily consistent but it is faster than the proposed implementation

I can explain the problem we're fixing... I think the real question here though is whether the problem we're fixing is a big enough problem that it need to be fixed, or just not worth worrying about.

The problem is that the specification of `Name.compareTo()` is so loose that it introduces a source of non-determinism in the compiler that is dependent on (for example) the order in which class files are read. That non-determinism then flows into `TypeSymbol.precedes()`.

So the real question is, do we really want `TypeSymbol.precedes()` potentially sorting `TypeSymbols` differently each time?

I can't really say definitively, because I'm not familiar with this method - so I'm just basing my opinion on the circumstantial evidence that there are 9 regression tests that are sensitive to this ordering, and also my overall general feeling that needless non-determinism is always a bad thing in computer software because it makes everything harder to debug.

The downside of this patch is a slower implementation of `TypeSymbol.precedes()` - again, I'm not familiar with this method so unsure how critical it is. Also, note that this downside may happen in the future regardless if/when the `Name` classes get refactored to just be `String` wrappers, etc.

So... I dunno. Your thoughts?

-------------

PR Comment: https://git.openjdk.org/jdk/pull/12843#issuecomment-1483505124