Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: improve mem reading in Memory #2667

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions rskj-core/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ ext {
// WARN: consider different setups and sure to not use GPG elements without checksums or without signature file
// in the remote repository (see https://github.com/gradle/gradle/security/advisories/GHSA-j6wc-xfg8-jx2j)
dependencies {
jmhImplementation project(path: ':rskj-core')
jmhImplementation "${jmhLibs.jmhCore}"
jmhImplementation "${jmhLibs.web3jCore}"
jmhImplementation "${libs.slf4jApiLib}"
Expand Down
62 changes: 62 additions & 0 deletions rskj-core/src/jmh/java/co/rsk/jmh/utils/BenchmarkByteUtil.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* This file is part of RskJ
* Copyright (C) 2024 RSK Labs Ltd.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

package co.rsk.jmh.utils;

import co.rsk.core.types.bytes.Bytes;
import co.rsk.jmh.utils.plan.DataPlan;
import org.openjdk.jmh.annotations.*;
import org.openjdk.jmh.runner.Runner;
import org.openjdk.jmh.runner.RunnerException;
import org.openjdk.jmh.runner.options.Options;
import org.openjdk.jmh.runner.options.OptionsBuilder;

import java.util.concurrent.TimeUnit;

@BenchmarkMode({Mode.Throughput})
@Warmup(iterations = 1, time = 5 /* secs */)
@Measurement(iterations = 3, time = 5 /* secs */)
@OutputTimeUnit(TimeUnit.MILLISECONDS)
public class BenchmarkByteUtil {

@Benchmark
public void toHexString_Bouncycastle(DataPlan plan) {
byte[] data = plan.getData();
int off = plan.getNextRand(data.length);
int len = plan.getNextRand(data.length - off);
org.bouncycastle.util.encoders.Hex.toHexString(data, off, len);
}

@Benchmark
public void toHexString_V2(DataPlan plan) {
Bytes bytes = plan.getBytes();
int bytesLen = bytes.length();
int off = plan.getNextRand(bytesLen);
int len = plan.getNextRand(bytesLen - off);
bytes.toHexString(off, len);
}

public static void main(String[] args) throws RunnerException {
Options opt = new OptionsBuilder()
.include(BenchmarkByteUtil.class.getName())
.forks(2)
.build();

new Runner(opt).run();
}
}
56 changes: 56 additions & 0 deletions rskj-core/src/jmh/java/co/rsk/jmh/utils/plan/DataPlan.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* This file is part of RskJ
* Copyright (C) 2024 RSK Labs Ltd.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

package co.rsk.jmh.utils.plan;

import co.rsk.core.types.bytes.Bytes;
import org.openjdk.jmh.annotations.Level;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.Setup;
import org.openjdk.jmh.annotations.State;

import java.util.Random;

@State(Scope.Benchmark)
public class DataPlan {

private final byte[] data = new byte[1024];

private Bytes bytes;

private Random random;

@Setup(Level.Trial)
public void doSetup() {
random = new Random(111);
random.nextBytes(data);
bytes = Bytes.of(data);
}

public byte[] getData() {
return data;
}

public Bytes getBytes() {
return bytes;
}

public int getNextRand(int bound) {
return random.nextInt(bound);
}
}
42 changes: 42 additions & 0 deletions rskj-core/src/main/java/co/rsk/core/types/bytes/BoundaryUtils.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* This file is part of RskJ
* Copyright (C) 2024 RSK Labs Ltd.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

package co.rsk.core.types.bytes;

public class BoundaryUtils {

private BoundaryUtils() { /* hidden */ }

public static void checkArraycopyParams(int srcLength, int srcPos, byte[] dest, int destPos, int length) {
if (length < 0) {
throw new IndexOutOfBoundsException("invalid 'length': " + length);
}
if (srcPos < 0 || Long.sum(srcPos, length) > srcLength) {
throw new IndexOutOfBoundsException("invalid 'srcPos' and/or 'length': [" + srcPos + ";" + length + ")");
}
if (destPos < 0 || Long.sum(destPos, length) > dest.length) {
throw new IndexOutOfBoundsException("invalid 'destPos' and/or 'length': [" + destPos + ";" + length + ")");
}
}

public static void checkArrayIndexParam(int srcLength, int index) {
if (index < 0 || index >= srcLength) {
throw new IndexOutOfBoundsException("invalid index: " + index);
}
}
}
11 changes: 0 additions & 11 deletions rskj-core/src/main/java/co/rsk/core/types/bytes/Bytes.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

package co.rsk.core.types.bytes;

import org.ethereum.util.ByteUtil;
import org.ethereum.util.FastByteComparisons;

import javax.annotation.Nonnull;
Expand Down Expand Up @@ -143,16 +142,6 @@ public void arraycopy(int srcPos, byte[] dest, int destPos, int length) {
System.arraycopy(byteArray, srcPos, dest, destPos, length);
}

@Override
public String toHexString() {
return ByteUtil.toHexString(byteArray);
}

@Override
public String toHexString(int off, int length) {
return ByteUtil.toHexString(byteArray, off, length);
}

@Nonnull
@Override
public byte[] asUnsafeByteArray() {
Expand Down
67 changes: 40 additions & 27 deletions rskj-core/src/main/java/co/rsk/core/types/bytes/BytesSlice.java
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,10 @@ default byte[] copyArrayOfRange(int from, int to) {
if (from < 0 || from > length()) {
throw new IndexOutOfBoundsException("invalid 'from': " + from);
}
int newLength = to - from;
if (newLength < 0) {
if (to < from) {
throw new IllegalArgumentException(from + " > " + to);
}
int newLength = to - from;
byte[] copy = new byte[newLength];
arraycopy(from, copy, 0, Math.min(length() - from, newLength));
return copy;
Expand All @@ -128,6 +128,42 @@ default Bytes copyBytes() {
default BytesSlice slice(int from, int to) {
return new BytesSliceImpl(this, from, to);
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about convert this class to an abstract class? I think it could give us various advantages:

  • We could make both equals and hashcode methods part of the class and not implemented as static.
  • We can force the usage of BoundaryUtils.checkArrayIndexParam(length(), index)
  • Stop using default methods for defining common implementations and facilitate the testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather use interface here. I guess it's quite similar to CharSequence interface that's being used for chars without concrete implementation of equals and hashCode

static boolean equals(BytesSlice a, BytesSlice b) {
if (a == b) {
return true;
}
if (a == null || b == null) {
return false;
}

int aLen = a.length();
if (b.length() != aLen) {
return false;
}

for (int i = 0; i < aLen; i++) {
if (a.byteAt(i) != b.byteAt(i)) {
return false;
}
}

return true;
}

static int hashCode(BytesSlice bytesSlice) {
if (bytesSlice == null) {
return 0;
}

int result = 1;
int len = bytesSlice.length();
for (int i = 0; i < len; i++) {
result = 31 * result + bytesSlice.byteAt(i);
}

return result;
}
}

class BytesSliceImpl implements BytesSlice {
Expand Down Expand Up @@ -161,39 +197,16 @@ public int length() {

@Override
public byte byteAt(int index) {
if (index < 0 || index >= length()) {
throw new IndexOutOfBoundsException("invalid index: " + index);
}
BoundaryUtils.checkArrayIndexParam(length(), index);
return originBytes.byteAt(from + index);
}

@Override
public void arraycopy(int srcPos, byte[] dest, int destPos, int length) {
if (length < 0) {
throw new IndexOutOfBoundsException("invalid 'length': " + length);
}
if (srcPos < 0 || srcPos + length > length()) {
throw new IndexOutOfBoundsException("invalid 'srcPos' and/or 'length': [" + srcPos + ";" + length + ")");
}
if (destPos < 0 || destPos + length > dest.length) {
throw new IndexOutOfBoundsException("invalid 'destPos' and/or 'length': [" + destPos + ";" + length + ")");
}
BoundaryUtils.checkArraycopyParams(length(), srcPos, dest, destPos, length);
originBytes.arraycopy(this.from + srcPos, dest, destPos, length);
}

@Override
public String toHexString(int off, int length) {
if (off < 0 || length < 0 || off + length > length()) {
throw new IndexOutOfBoundsException("invalid 'off' and/or 'length': " + off + "; " + length);
}
return originBytes.toHexString(from + off, length);
}

@Override
public String toHexString() {
return toHexString(0, length());
}

@Override
public String toString() {
return toPrintableString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

package co.rsk.core.types.bytes;

import org.ethereum.util.ByteUtil;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.util.Objects;
Expand Down Expand Up @@ -62,17 +64,49 @@ default String toJsonHexFormattedString() {
return toPrintableString(SIMPLE_JSON_HEX_FORMATTER);
}

String toHexString(int off, int length);
default String toHexString(int off, int length) {
return toHexStringV2(off, length);
}

default String toHexString() {
return toHexString(0, length());
}

/**
* This is a bit optimized version of {@link ByteUtil#toHexString(byte[], int, int)},
* which does not use a third-party library.
*
* @param offset the start index of the bytes to be converted to hexadecimal.
* It must be non-negative and less than the length of the bytes.
* Otherwise, an {@link IndexOutOfBoundsException} will be thrown.
* @param length the number of bytes to be converted to hexadecimal.
* It must be non-negative and less than the length of the bytes.
* Otherwise, an {@link IndexOutOfBoundsException} will be thrown.
*
* @return the hexadecimal representation of the bytes in the range of {@code offset} and {@code length}.
*/
default String toHexStringV2(int offset, int length) {
if (offset < 0 || length < 0 || Long.sum(offset, length) > length()) {
throw new IndexOutOfBoundsException("invalid 'offset' and/or 'length': " + offset + "; " + length);
}

String toHexString();
int endIndex = offset + length;
StringBuilder sb = new StringBuilder(length * 2);
for (int i = offset; i < endIndex; i++) {
Fixed Show fixed Hide fixed
byte b = byteAt(i);
sb.append(Character.forDigit((b >> 4) & 0xF, 16));
sb.append(Character.forDigit((b & 0xF), 16));
Vovchyk marked this conversation as resolved.
Show resolved Hide resolved
}
return sb.toString();
}
}

class PrintableBytesHexFormatter implements PrintableBytes.Formatter<HexPrintableBytes> {

@Override
public String toFormattedString(@Nonnull HexPrintableBytes printableBytes, int off, int length) {
int bytesLen = Objects.requireNonNull(printableBytes).length();
if (off + length > bytesLen) {
if (off < 0 || length < 0 || Long.sum(off, length) > bytesLen) {
throw new IndexOutOfBoundsException("invalid 'off' and/or 'length': " + off + "; " + length);
}

Expand Down
8 changes: 7 additions & 1 deletion rskj-core/src/main/java/org/ethereum/crypto/HashUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
package org.ethereum.crypto;

import co.rsk.core.RskAddress;
import co.rsk.core.types.bytes.Bytes;
import co.rsk.core.types.bytes.BytesSlice;
import org.bouncycastle.crypto.Digest;
import org.bouncycastle.crypto.digests.RIPEMD160Digest;
import org.ethereum.crypto.cryptohash.Keccak256;
Expand Down Expand Up @@ -59,6 +61,10 @@ public static byte[] sha256(byte[] input) {
}

public static byte[] keccak256(byte[] input) {
return keccak256(Bytes.of(input));
}

public static byte[] keccak256(BytesSlice input) {
Keccak256 digest = new Keccak256();
digest.update(input);
return digest.digest();
Expand Down Expand Up @@ -125,7 +131,7 @@ public static byte[] calcNewAddr(byte[] addr, byte[] nonce) {
* @param salt - salt to make different result addresses
* @return new address
*/
public static byte[] calcSaltAddr(RskAddress senderAddress, byte[] initCode, byte[] salt) {
public static byte[] calcSaltAddr(RskAddress senderAddress, BytesSlice initCode, byte[] salt) {
// 0xff is of length 1
// keccak-256 of the address is of length 32
// Then we add the lengths of the senderAddress and the salt
Expand Down
Loading
Loading