Merge pull request #84 from catbref/ByteArray

Improvements to ByteArray to leverage Java 11 'native' Arrays methods
This commit is contained in:
CalDescent 2022-04-14 21:30:59 +01:00 committed by GitHub
commit 58a0ac74d2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 96 additions and 69 deletions

View File

@ -675,7 +675,7 @@ public class Controller extends Thread {
public static final Predicate<Peer> hasInferiorChainTip = peer -> {
final PeerChainTipData peerChainTipData = peer.getChainTipData();
final List<ByteArray> inferiorChainTips = Synchronizer.getInstance().inferiorChainSignatures;
return peerChainTipData == null || peerChainTipData.getLastBlockSignature() == null || inferiorChainTips.contains(new ByteArray(peerChainTipData.getLastBlockSignature()));
return peerChainTipData == null || peerChainTipData.getLastBlockSignature() == null || inferiorChainTips.contains(ByteArray.wrap(peerChainTipData.getLastBlockSignature()));
};
public static final Predicate<Peer> hasOldVersion = peer -> {
@ -1203,7 +1203,7 @@ public class Controller extends Thread {
byte[] signature = getBlockMessage.getSignature();
this.stats.getBlockMessageStats.requests.incrementAndGet();
ByteArray signatureAsByteArray = new ByteArray(signature);
ByteArray signatureAsByteArray = ByteArray.wrap(signature);
CachedBlockMessage cachedBlockMessage = this.blockMessageCache.get(signatureAsByteArray);
int blockCacheSize = Settings.getInstance().getBlockCacheSize();
@ -1283,7 +1283,7 @@ public class Controller extends Thread {
if (getChainHeight() - blockData.getHeight() <= blockCacheSize) {
this.stats.getBlockMessageStats.cacheFills.incrementAndGet();
this.blockMessageCache.put(new ByteArray(blockData.getSignature()), blockMessage);
this.blockMessageCache.put(ByteArray.wrap(blockData.getSignature()), blockMessage);
}
} catch (DataException e) {
LOGGER.error(String.format("Repository issue while send block %s to peer %s", Base58.encode(signature), peer), e);

View File

@ -314,7 +314,7 @@ public class Synchronizer extends Thread {
case INFERIOR_CHAIN: {
// Update our list of inferior chain tips
ByteArray inferiorChainSignature = new ByteArray(peer.getChainTipData().getLastBlockSignature());
ByteArray inferiorChainSignature = ByteArray.wrap(peer.getChainTipData().getLastBlockSignature());
if (!inferiorChainSignatures.contains(inferiorChainSignature))
inferiorChainSignatures.add(inferiorChainSignature);
@ -343,7 +343,7 @@ public class Synchronizer extends Thread {
// fall-through...
case NOTHING_TO_DO: {
// Update our list of inferior chain tips
ByteArray inferiorChainSignature = new ByteArray(peer.getChainTipData().getLastBlockSignature());
ByteArray inferiorChainSignature = ByteArray.wrap(peer.getChainTipData().getLastBlockSignature());
if (!inferiorChainSignatures.contains(inferiorChainSignature))
inferiorChainSignatures.add(inferiorChainSignature);
@ -419,7 +419,7 @@ public class Synchronizer extends Thread {
public void addInferiorChainSignature(byte[] inferiorSignature) {
// Update our list of inferior chain tips
ByteArray inferiorChainSignature = new ByteArray(inferiorSignature);
ByteArray inferiorChainSignature = ByteArray.wrap(inferiorSignature);
if (!inferiorChainSignatures.contains(inferiorChainSignature))
inferiorChainSignatures.add(inferiorChainSignature);
}

View File

@ -402,7 +402,7 @@ public class TradeBot implements Listener {
long now = NTP.getTime();
long newExpiry = generateExpiry(now);
ByteArray pubkeyByteArray = ByteArray.of(tradeNativeAccount.getPublicKey());
ByteArray pubkeyByteArray = ByteArray.wrap(tradeNativeAccount.getPublicKey());
// If map entry's timestamp is missing, or within early renewal period, use the new expiry - otherwise use existing timestamp.
synchronized (this.ourTradePresenceTimestampsByPubkey) {
@ -489,7 +489,7 @@ public class TradeBot implements Listener {
int knownCount = entriesUnknownToPeer.size();
for (TradePresenceData peersTradePresence : peersTradePresences) {
ByteArray pubkeyByteArray = ByteArray.of(peersTradePresence.getPublicKey());
ByteArray pubkeyByteArray = ByteArray.wrap(peersTradePresence.getPublicKey());
TradePresenceData ourEntry = entriesUnknownToPeer.get(pubkeyByteArray);
@ -546,7 +546,7 @@ public class TradeBot implements Listener {
continue;
}
ByteArray pubkeyByteArray = ByteArray.of(peersTradePresence.getPublicKey());
ByteArray pubkeyByteArray = ByteArray.wrap(peersTradePresence.getPublicKey());
// Ignore if we've previously verified this timestamp+publickey combo or sent timestamp is older
TradePresenceData existingTradeData = this.safeAllTradePresencesByPubkey.get(pubkeyByteArray);
@ -589,7 +589,7 @@ public class TradeBot implements Listener {
continue;
}
ByteArray atCodeHash = new ByteArray(atData.getCodeHash());
ByteArray atCodeHash = ByteArray.wrap(atData.getCodeHash());
Supplier<ACCT> acctSupplier = acctSuppliersByCodeHash.get(atCodeHash);
if (acctSupplier == null) {
LOGGER.trace("Ignoring trade presence {} from peer {} as AT isn't a known ACCT?",
@ -642,7 +642,7 @@ public class TradeBot implements Listener {
public void bridgePresence(long timestamp, byte[] publicKey, byte[] signature, String atAddress) {
long expiry = generateExpiry(timestamp);
ByteArray pubkeyByteArray = ByteArray.of(publicKey);
ByteArray pubkeyByteArray = ByteArray.wrap(publicKey);
TradePresenceData fakeTradePresenceData = new TradePresenceData(expiry, publicKey, signature, atAddress);

View File

@ -62,7 +62,7 @@ public enum SupportedBlockchain {
private static final Map<ByteArray, Supplier<ACCT>> supportedAcctsByCodeHash = Arrays.stream(SupportedBlockchain.values())
.map(supportedBlockchain -> supportedBlockchain.supportedAccts)
.flatMap(List::stream)
.collect(Collectors.toUnmodifiableMap(triple -> new ByteArray(triple.getB()), Triple::getC));
.collect(Collectors.toUnmodifiableMap(triple -> ByteArray.wrap(triple.getB()), Triple::getC));
private static final Map<String, Supplier<ACCT>> supportedAcctsByName = Arrays.stream(SupportedBlockchain.values())
.map(supportedBlockchain -> supportedBlockchain.supportedAccts)
@ -94,7 +94,7 @@ public enum SupportedBlockchain {
return getAcctMap();
return blockchain.supportedAccts.stream()
.collect(Collectors.toUnmodifiableMap(triple -> new ByteArray(triple.getB()), Triple::getC));
.collect(Collectors.toUnmodifiableMap(triple -> ByteArray.wrap(triple.getB()), Triple::getC));
}
public static Map<ByteArray, Supplier<ACCT>> getFilteredAcctMap(String specificBlockchain) {
@ -109,7 +109,7 @@ public enum SupportedBlockchain {
}
public static ACCT getAcctByCodeHash(byte[] codeHash) {
ByteArray wrappedCodeHash = new ByteArray(codeHash);
ByteArray wrappedCodeHash = ByteArray.wrap(codeHash);
Supplier<ACCT> acctInstanceSupplier = supportedAcctsByCodeHash.get(wrappedCodeHash);

View File

@ -185,7 +185,7 @@ public class PresenceTransaction extends Transaction {
String signerAddress = Crypto.toAddress(this.transactionData.getCreatorPublicKey());
for (ATData atData : atsData) {
ByteArray atCodeHash = new ByteArray(atData.getCodeHash());
ByteArray atCodeHash = ByteArray.wrap(atData.getCodeHash());
Supplier<ACCT> acctSupplier = acctSuppliersByCodeHash.get(atCodeHash);
if (acctSupplier == null)
continue;

View File

@ -8,12 +8,16 @@ public class ByteArray implements Comparable<ByteArray> {
private int hash;
public final byte[] value;
public ByteArray(byte[] value) {
this.value = Objects.requireNonNull(value);
private ByteArray(byte[] value) {
this.value = value;
}
public static ByteArray of(byte[] value) {
return new ByteArray(value);
public static ByteArray wrap(byte[] value) {
return new ByteArray(Objects.requireNonNull(value));
}
public static ByteArray copyOf(byte[] value) {
return new ByteArray(Arrays.copyOf(value, value.length));
}
@Override
@ -36,12 +40,7 @@ public class ByteArray implements Comparable<ByteArray> {
byte[] val = this.value;
if (h == 0 && val.length > 0) {
h = 1;
for (int i = 0; i < val.length; ++i)
h = 31 * h + val[i];
this.hash = h;
this.hash = h = Arrays.hashCode(val);
}
return h;
}
@ -53,24 +52,7 @@ public class ByteArray implements Comparable<ByteArray> {
}
public int compareToPrimitive(byte[] otherValue) {
byte[] val = this.value;
if (val.length < otherValue.length)
return -1;
if (val.length > otherValue.length)
return 1;
for (int i = 0; i < val.length; ++i) {
int a = val[i] & 0xFF;
int b = otherValue[i] & 0xFF;
if (a < b)
return -1;
if (a > b)
return 1;
}
return 0;
return Arrays.compareUnsigned(this.value, otherValue);
}
public String toString() {

View File

@ -32,19 +32,40 @@ public class ByteArrayTests {
private static void fillMap(Map<ByteArray, String> map) {
for (byte[] testValue : testValues)
map.put(new ByteArray(testValue), String.valueOf(map.size()));
map.put(ByteArray.wrap(testValue), String.valueOf(map.size()));
}
private static byte[] dup(byte[] value) {
return Arrays.copyOf(value, value.length);
}
@Test
@SuppressWarnings("unlikely-arg-type")
public void testOriginatingIssue() {
Map<byte[], String> testMap = new HashMap<>();
byte[] someValue = testValues.get(3);
testMap.put(someValue, "someValue");
byte[] copiedValue = dup(someValue);
// Show that a byte[] with same values is not found
System.out.printf("byte[] hashCode: 0x%08x%n", someValue.hashCode());
System.out.printf("duplicated byte[] hashCode: 0x%08x%n", copiedValue.hashCode());
/*
* Unfortunately this doesn't work because HashMap::containsKey compares hashCodes first,
* followed by object references, and copiedValue.hashCode() will never match someValue.hashCode().
*/
assertFalse("byte[] with same values, but difference reference, not found", testMap.containsKey(copiedValue));
}
@Test
public void testSameContentReference() {
// Create two objects, which will have different references, but same content.
// Create two objects, which will have different references, but same content references.
byte[] testValue = testValues.get(0);
ByteArray ba1 = new ByteArray(testValue);
ByteArray ba2 = new ByteArray(testValue);
ByteArray ba1 = ByteArray.wrap(testValue);
ByteArray ba2 = ByteArray.wrap(testValue);
// Confirm JVM-assigned references are different
assertNotSame(ba1, ba2);
@ -58,13 +79,31 @@ public class ByteArrayTests {
}
@Test
public void testSameContentValue() {
// Create two objects, which will have different references, but same content.
public void testSameWrappedContentValue() {
// Create two objects, which will have different references, and different content references, but same content values.
byte[] testValue = testValues.get(0);
ByteArray ba1 = new ByteArray(testValue);
ByteArray ba1 = ByteArray.wrap(testValue);
byte[] copiedValue = dup(testValue);
ByteArray ba2 = new ByteArray(copiedValue);
ByteArray ba2 = ByteArray.wrap(copiedValue);
// Confirm JVM-assigned references are different
assertNotSame(ba1, ba2);
// Confirm "equals" works as intended
assertTrue("equals did not return true", ba1.equals(ba2));
assertEquals("ba1 not equal to ba2", ba1, ba2);
// Confirm "hashCode" results match
assertEquals("hashCodes do not match", ba1.hashCode(), ba2.hashCode());
}
@Test
public void testSameCopiedContentValue() {
// Create two objects, which will have different references, and different content references, but same content values.
byte[] testValue = testValues.get(0);
ByteArray ba1 = ByteArray.wrap(testValue);
ByteArray ba2 = ByteArray.copyOf(testValue);
// Confirm JVM-assigned references are different
assertNotSame(ba1, ba2);
@ -81,13 +120,17 @@ public class ByteArrayTests {
@SuppressWarnings("unlikely-arg-type")
public void testCompareBoxedWithPrimitive() {
byte[] testValue = testValues.get(0);
ByteArray ba1 = new ByteArray(testValue);
ByteArray wrappedByteArray = ByteArray.wrap(testValue);
byte[] copiedValue = dup(testValue);
ByteArray copiedByteArray = ByteArray.copyOf(copiedValue);
// Confirm "equals" works as intended
assertTrue("equals did not return true", ba1.equals(copiedValue));
assertEquals("boxed not equal to primitive", ba1, copiedValue);
assertTrue("equals did not return true", wrappedByteArray.equals(copiedValue));
assertEquals("boxed not equal to primitive", wrappedByteArray, copiedValue);
assertTrue("equals did not return true", copiedByteArray.equals(testValue));
assertEquals("boxed not equal to primitive", copiedByteArray, testValue);
}
@Test
@ -98,7 +141,7 @@ public class ByteArrayTests {
// Create new ByteArray object with an existing value.
byte[] copiedValue = dup(testValues.get(3));
ByteArray ba = new ByteArray(copiedValue);
ByteArray ba = ByteArray.wrap(copiedValue);
// Confirm object can be found in map
assertTrue("ByteArray not found in map", testMap.containsKey(ba));
@ -120,7 +163,7 @@ public class ByteArrayTests {
// Create new ByteArray object with an existing value.
byte[] copiedValue = dup(testValues.get(3));
ByteArray ba = new ByteArray(copiedValue);
ByteArray ba = ByteArray.wrap(copiedValue);
// Confirm object can be found in map
assertTrue("ByteArray not found in map", testMap.containsKey(ba));
@ -128,7 +171,7 @@ public class ByteArrayTests {
assertTrue("boxed not equal to primitive", ba.equals(copiedValue));
/*
* Unfortunately this doesn't work because TreeMap::containsKey(x) wants to cast x to
* Unfortunately this doesn't work because TreeMap::containsKey(byte[]) wants to cast byte[] to
* Comparable<? super ByteArray> and byte[] does not fit <? super ByteArray>
* so this throws a ClassCastException.
*/
@ -145,7 +188,7 @@ public class ByteArrayTests {
public void testArrayListContains() {
// Create new ByteArray object with an existing value.
byte[] copiedValue = dup(testValues.get(3));
ByteArray ba = new ByteArray(copiedValue);
ByteArray ba = ByteArray.wrap(copiedValue);
// Confirm object can be found in list
assertTrue("ByteArray not found in map", testValues.contains(ba));
@ -154,7 +197,7 @@ public class ByteArrayTests {
/*
* Unfortunately this doesn't work because ArrayList::contains performs
* copiedValue.equals(x) for each x in testValues, and byte[].equals()
* copiedValue.equals(byte[]) for each byte[] in testValues, and byte[].equals()
* simply compares object references, so will never match any ByteArray.
*/
assertFalse("Primitive shouldn't be found in ArrayList", testValues.contains(copiedValue));
@ -163,23 +206,25 @@ public class ByteArrayTests {
@Test
public void debugBoxedVersusPrimitive() {
byte[] testValue = testValues.get(0);
ByteArray ba1 = new ByteArray(testValue);
ByteArray ba1 = ByteArray.wrap(testValue);
byte[] copiedValue = dup(testValue);
System.out.println(String.format("Primitive hashCode: 0x%08x", testValue.hashCode()));
System.out.println(String.format("Boxed hashCode: 0x%08x", ba1.hashCode()));
System.out.println(String.format("Duplicated primitive hashCode: 0x%08x", copiedValue.hashCode()));
System.out.printf("Primitive hashCode: 0x%08x%n", testValue.hashCode());
System.out.printf("Boxed hashCode: 0x%08x%n", ba1.hashCode());
System.out.printf("Duplicated primitive hashCode: 0x%08x%n", copiedValue.hashCode());
}
@Test
public void testCompareTo() {
ByteArray testValue0 = new ByteArray(new byte[] { 0x00 });
ByteArray testValue1 = new ByteArray(new byte[] { 0x01 });
ByteArray testValue0 = ByteArray.wrap(new byte[] { 0x00 });
ByteArray testValue1 = ByteArray.wrap(new byte[] { 0x01 });
ByteArray testValueFf = ByteArray.wrap(new byte[] {(byte) 0xFF});
assertEquals("0 should be the same as 0", 0, testValue0.compareTo(testValue0));
assertEquals("0 should be before 1", -1, testValue0.compareTo(testValue1));
assertEquals("1 should be after 0", 1, testValue1.compareTo(testValue0));
assertTrue("0 should be the same as 0", testValue0.compareTo(testValue0) == 0);
assertTrue("0 should be before 1", testValue0.compareTo(testValue1) < 0);
assertTrue("1 should be after 0", testValue1.compareTo(testValue0) > 0);
assertTrue("FF should be after 0", testValueFf.compareTo(testValue0) > 0);
}
}

View File

@ -55,7 +55,7 @@ public class TradeBotPresenceTests {
@Test
public void testEnforceLatestTimestamp() {
ByteArray pubkeyByteArray = ByteArray.of("publickey".getBytes(StandardCharsets.UTF_8));
ByteArray pubkeyByteArray = ByteArray.wrap("publickey".getBytes(StandardCharsets.UTF_8));
Map<ByteArray, Long> timestampsByPublicKey = new HashMap<>();