RFR: 8312089: Simplify and modernize equals, hashCode, and compareTo in java.nio and implementation code

[Alan Bateman]

On Fri, 14 Jul 2023 12:06:29 GMT, Pavel Rappo <prappo at openjdk.org> wrote:

> Please review this PR to use modern APIs and language features to simplify equals, hashCode, and compareTo for in java.nio and implementation code.
> 
> Please note, test results are pending.
> 
> Additional notes:
> 
> * This PR saves a volatile read in java.nio.file.attribute.AclEntry.hashCode. Not that it's too important, but worth noting because of rearrangements.
> 
> * java.nio.charset.Charset#compareTo seems **inconsistent** with equals. If so, I cannot see where that inconsistency is specified.
> 
> * Is this a **bug** in sun.nio.ch.FileKey#hashCode? Tell me if not, I'll revert it.
> 
> * This PR simplifies the tail of java.nio.file.attribute.FileTime.compareTo. Unless I'm missing something, that comment in source above the affected lines **seems** not to prohibit such a simplification.
> 
> * sun.nio.fs.UnixFileStore#hashCode does not include entry.name(). While it's not wrong, I wonder if it was on purpose.
> 
> * Despite its title, this PR also and opportunistically refactors sun.nio.fs.UnixPath.endsWith.

src/java.base/share/classes/java/nio/charset/Charset.java line 957:

> 955:      *         is less than, equal to, or greater than the specified charset
> 956:      */
> 957:     @Override

This PR adds `@Override` to a subset of the overridden methods, probably best to add it to all overridden methods so we don't end up with a mix.

src/java.base/share/classes/java/nio/charset/Charset.java line 963:

> 961: 
> 962:     /**
> 963:      * {@return a hashcode for this charset}

If you changing this then probably better to use "the hashcode".

src/java.base/share/classes/java/nio/file/attribute/FileTime.java line 368:

> 366:                 return cmp;
> 367:             }
> 368:             return Long.compare(toExcessNanos(days), other.toExcessNanos(daysOther));

Maybe a coin toss, but I think the original code was a bit clearer.

src/java.base/unix/classes/sun/nio/fs/UnixFileKey.java line 53:

> 51:         if (!(obj instanceof UnixFileKey other))
> 52:             return false;
> 53:         return (this.st_dev == other.st_dev) && (this.st_ino == other.st_ino);

Style-wise, I think usages like this are a bit easier to read.

        return obj instanceof UnixFileKey other
                && (this.st_dev == other.st_dev)
                && (this.st_ino == other.st_ino);

src/java.base/unix/classes/sun/nio/fs/UnixPath.java line 712:

> 710:         int thatPos = that.offsets[0];
> 711:         return Arrays.equals(this.path, thisPos, thisLen, that.path, thatPos,
> 712:                 thatLen);

Might be cleaner to not put a line break here.

src/java.base/unix/classes/sun/nio/fs/UnixPath.java line 731:

> 729:         if (h == 0) {
> 730:             h = ArraysSupport.vectorizedHashCode(path, 0, path.length, 0,
> 731:                     ArraysSupport.T_BOOLEAN);

Can you make this `/* unsigned bytes */ ArraysSupport.T_BOOLEAN`, or `ArraysSupport.T_BOOLEAN); // unsigned bytes`,  only because T_BOOLEAN isn't always obvious as use-sites.

src/java.base/windows/classes/sun/nio/ch/FileKey.java line 52:

> 50:         return (int)(dwVolumeSerialNumber ^ (dwVolumeSerialNumber >>> 32)) +
> 51:                (int)(nFileIndexHigh ^ (nFileIndexHigh >>> 32)) +
> 52:                (int)(nFileIndexLow ^ (nFileIndexLow >>> 32));

Well spotted, probably a cut and paste error at some point, but benign, the resulting hash code is still okay.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14886#discussion_r1263667696
PR Review Comment: https://git.openjdk.org/jdk/pull/14886#discussion_r1263666839
PR Review Comment: https://git.openjdk.org/jdk/pull/14886#discussion_r1263677841
PR Review Comment: https://git.openjdk.org/jdk/pull/14886#discussion_r1263671563
PR Review Comment: https://git.openjdk.org/jdk/pull/14886#discussion_r1263679533
PR Review Comment: https://git.openjdk.org/jdk/pull/14886#discussion_r1263684952
PR Review Comment: https://git.openjdk.org/jdk/pull/14886#discussion_r1263680738