From 3115f80fde65baccf179937ddcd63ff1ec319e2d Mon Sep 17 00:00:00 2001 From: skrzypo987 Date: Fri, 12 Aug 2022 08:48:19 +0300 Subject: [PATCH 1/3] Rename variables --- .../trino/operator/join/BigintPagesHash.java | 32 +++++++++---------- .../trino/operator/join/DefaultPagesHash.java | 30 ++++++++--------- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/operator/join/BigintPagesHash.java b/core/trino-main/src/main/java/io/trino/operator/join/BigintPagesHash.java index 7ffcd20422c6..5836c9813102 100644 --- a/core/trino-main/src/main/java/io/trino/operator/join/BigintPagesHash.java +++ b/core/trino-main/src/main/java/io/trino/operator/join/BigintPagesHash.java @@ -49,7 +49,7 @@ public final class BigintPagesHash private final PagesHashStrategy pagesHashStrategy; private final int mask; - private final int[] key; + private final int[] keys; private final long[] values; private final long size; @@ -77,9 +77,9 @@ public BigintPagesHash( int hashSize = hashArraySizeSupplier.getHashArraySize(addresses.size()); mask = hashSize - 1; - key = new int[hashSize]; + keys = new int[hashSize]; values = new long[addresses.size()]; - Arrays.fill(key, -1); + Arrays.fill(keys, -1); // We will process addresses in batches, to improve spatial and temporal memory locality int positionsInStep = Math.min(addresses.size() + 1, (int) CACHE_SIZE.toBytes() / Integer.SIZE); @@ -91,13 +91,13 @@ public BigintPagesHash( int stepSize = stepEndPosition - stepBeginPosition; // index pages - for (int position = 0; position < stepSize; position++) { - int realPosition = position + stepBeginPosition; - if (isPositionNull(realPosition)) { + for (int batchIndex = 0; batchIndex < stepSize; batchIndex++) { + int addressIndex = batchIndex + stepBeginPosition; + if (isPositionNull(addressIndex)) { continue; } - long address = addresses.getLong(realPosition); + long address = addresses.getLong(addressIndex); int blockIndex = decodeSliceIndex(address); int blockPosition = decodePosition(address); long value = joinChannelBlocks.get(blockIndex).getLong(blockPosition, 0); @@ -105,12 +105,12 @@ public BigintPagesHash( int pos = getHashPosition(value, mask); // look for an empty slot or a slot containing this key - while (key[pos] != -1) { - int currentKey = key[pos]; + while (keys[pos] != -1) { + int currentKey = keys[pos]; if (value == values[currentKey]) { // found a slot for this key // link the new key position to the current key position - realPosition = positionLinks.link(realPosition, currentKey); + addressIndex = positionLinks.link(addressIndex, currentKey); // key[pos] updated outside of this loop break; @@ -120,13 +120,13 @@ public BigintPagesHash( hashCollisionsLocal++; } - key[pos] = realPosition; - values[realPosition] = value; + keys[pos] = addressIndex; + values[addressIndex] = value; } } size = sizeOf(addresses.elements()) + pagesHashStrategy.getSizeInBytes() + - sizeOf(key) + sizeOf(values); + sizeOf(keys) + sizeOf(values); hashCollisions = hashCollisionsLocal; expectedHashCollisions = estimateNumberOfHashCollisions(addresses.size(), hashSize); } @@ -167,9 +167,9 @@ public int getAddressIndex(int position, Page hashChannelsPage) long value = hashChannelsPage.getBlock(0).getLong(position, 0); int pos = getHashPosition(value, mask); - while (key[pos] != -1) { - if (value == values[key[pos]]) { - return key[pos]; + while (keys[pos] != -1) { + if (value == values[keys[pos]]) { + return keys[pos]; } // increment position and mask to handler wrap around pos = (pos + 1) & mask; diff --git a/core/trino-main/src/main/java/io/trino/operator/join/DefaultPagesHash.java b/core/trino-main/src/main/java/io/trino/operator/join/DefaultPagesHash.java index 7d1e227d190f..72ba8012d3f5 100644 --- a/core/trino-main/src/main/java/io/trino/operator/join/DefaultPagesHash.java +++ b/core/trino-main/src/main/java/io/trino/operator/join/DefaultPagesHash.java @@ -46,7 +46,7 @@ public final class DefaultPagesHash private final PagesHashStrategy pagesHashStrategy; private final int mask; - private final int[] key; + private final int[] keys; private final long size; // Native array of hashes for faster collisions resolution compared @@ -69,8 +69,8 @@ public DefaultPagesHash( int hashSize = hashArraySizeSupplier.getHashArraySize(addresses.size()); mask = hashSize - 1; - key = new int[hashSize]; - Arrays.fill(key, -1); + keys = new int[hashSize]; + Arrays.fill(keys, -1); positionToHashes = new byte[addresses.size()]; @@ -87,11 +87,11 @@ public DefaultPagesHash( // First extract all hashes from blocks to native array. // Somehow having this as a separate loop is much faster compared // to extracting hashes on the fly in the loop below. - for (int position = 0; position < stepSize; position++) { - int realPosition = position + stepBeginPosition; - long hash = readHashPosition(realPosition); - positionToFullHashes[position] = hash; - positionToHashes[realPosition] = (byte) hash; + for (int batchIndex = 0; batchIndex < stepSize; batchIndex++) { + int addressIndex = batchIndex + stepBeginPosition; + long hash = readHashPosition(addressIndex); + positionToFullHashes[batchIndex] = hash; + positionToHashes[addressIndex] = (byte) hash; } // index pages @@ -105,8 +105,8 @@ public DefaultPagesHash( int pos = getHashPosition(hash, mask); // look for an empty slot or a slot containing this key - while (key[pos] != -1) { - int currentKey = key[pos]; + while (keys[pos] != -1) { + int currentKey = keys[pos]; if (((byte) hash) == positionToHashes[currentKey] && positionEqualsPositionIgnoreNulls(currentKey, realPosition)) { // found a slot for this key // link the new key position to the current key position @@ -120,12 +120,12 @@ public DefaultPagesHash( hashCollisionsLocal++; } - key[pos] = realPosition; + keys[pos] = realPosition; } } size = sizeOf(addresses.elements()) + pagesHashStrategy.getSizeInBytes() + - sizeOf(key) + sizeOf(positionToHashes); + sizeOf(keys) + sizeOf(positionToHashes); hashCollisions = hashCollisionsLocal; expectedHashCollisions = estimateNumberOfHashCollisions(addresses.size(), hashSize); } @@ -165,9 +165,9 @@ public int getAddressIndex(int rightPosition, Page hashChannelsPage, long rawHas { int pos = getHashPosition(rawHash, mask); - while (key[pos] != -1) { - if (positionEqualsCurrentRowIgnoreNulls(key[pos], (byte) rawHash, rightPosition, hashChannelsPage)) { - return key[pos]; + while (keys[pos] != -1) { + if (positionEqualsCurrentRowIgnoreNulls(keys[pos], (byte) rawHash, rightPosition, hashChannelsPage)) { + return keys[pos]; } // increment position and mask to handler wrap around pos = (pos + 1) & mask; From fca3243e5c72c7793610a159ff0625ee27488277 Mon Sep 17 00:00:00 2001 From: skrzypo987 Date: Fri, 12 Aug 2022 08:49:08 +0300 Subject: [PATCH 2/3] Change comment into a javadoc --- .../main/java/io/trino/operator/join/BigintPagesHash.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/operator/join/BigintPagesHash.java b/core/trino-main/src/main/java/io/trino/operator/join/BigintPagesHash.java index 5836c9813102..7edd8958d15b 100644 --- a/core/trino-main/src/main/java/io/trino/operator/join/BigintPagesHash.java +++ b/core/trino-main/src/main/java/io/trino/operator/join/BigintPagesHash.java @@ -35,9 +35,11 @@ import static java.lang.Math.toIntExact; import static java.util.Objects.requireNonNull; -// This implementation assumes: -// -There is only one join channel and it is of type bigint -// -arrays used in the hash are always a power of 2. +/** + * This implementation assumes: + * -There is only one join channel and it is of type bigint + * -arrays used in the hash are always a power of 2. + */ public final class BigintPagesHash implements PagesHash { From c2203b583944e311e6149332ae1fabdfb413d786 Mon Sep 17 00:00:00 2001 From: skrzypo987 Date: Wed, 17 Aug 2022 12:37:15 +0300 Subject: [PATCH 3/3] Improve comment --- .../src/main/java/io/trino/operator/join/DefaultPagesHash.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/trino-main/src/main/java/io/trino/operator/join/DefaultPagesHash.java b/core/trino-main/src/main/java/io/trino/operator/join/DefaultPagesHash.java index 72ba8012d3f5..f9b748bac60d 100644 --- a/core/trino-main/src/main/java/io/trino/operator/join/DefaultPagesHash.java +++ b/core/trino-main/src/main/java/io/trino/operator/join/DefaultPagesHash.java @@ -74,7 +74,7 @@ public DefaultPagesHash( positionToHashes = new byte[addresses.size()]; - // We will process addresses in batches, to save memory on array of hashes. + // We will process addresses in batches, to save memory on array of hashes and improve memory locality. int positionsInStep = Math.min(addresses.size() + 1, (int) CACHE_SIZE.toBytes() / Integer.SIZE); long[] positionToFullHashes = new long[positionsInStep]; long hashCollisionsLocal = 0;