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

Migrate Operand Stack to Growing Stack Pool #6068

Merged
merged 4 commits into from
Oct 25, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 2 additions & 2 deletions evm/src/main/java/org/hyperledger/besu/evm/EVM.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
import org.hyperledger.besu.evm.gascalculator.GasCalculator;
import org.hyperledger.besu.evm.internal.CodeCache;
import org.hyperledger.besu.evm.internal.EvmConfiguration;
import org.hyperledger.besu.evm.internal.FixedStack.OverflowException;
import org.hyperledger.besu.evm.internal.FixedStack.UnderflowException;
import org.hyperledger.besu.evm.internal.OverflowException;
import org.hyperledger.besu.evm.internal.UnderflowException;
import org.hyperledger.besu.evm.operation.AddModOperation;
import org.hyperledger.besu.evm.operation.AddOperation;
import org.hyperledger.besu.evm.operation.AndOperation;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.evm.Code;
import org.hyperledger.besu.evm.code.CodeSection;
import org.hyperledger.besu.evm.internal.FixedStack.UnderflowException;
import org.hyperledger.besu.evm.internal.MemoryEntry;
import org.hyperledger.besu.evm.internal.OperandStack;
import org.hyperledger.besu.evm.internal.ReturnStack;
import org.hyperledger.besu.evm.internal.StorageEntry;
import org.hyperledger.besu.evm.internal.UnderflowException;
import org.hyperledger.besu.evm.log.Log;
import org.hyperledger.besu.evm.operation.Operation;
import org.hyperledger.besu.evm.worldstate.WorldUpdater;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
import java.util.Arrays;

/**
* An operand stack for the Ethereum Virtual machine (EVM).
* An operand stack for the Ethereum Virtual machine (EVM). This stack allocates the entire stack at
* initialization time.
*
* <p>The operand stack is responsible for storing the current operands that the EVM can execute. It
* is assumed to have a fixed size.
Expand All @@ -35,30 +36,6 @@ public class FixedStack<T> {

private int top;

/** The Underflow exception. */
public static class UnderflowException extends RuntimeException {
// Don't create a stack trace since these are not "errors" per say but are using exceptions to
// throw rare control flow conditions (EVM stack overflow) that are expected to be seen in
// normal
// operations.
@Override
public synchronized Throwable fillInStackTrace() {
return this;
}
}

/** The Overflow exception. */
public static class OverflowException extends RuntimeException {
// Don't create a stack trace since these are not "errors" per say but are using exceptions to
// throw rare control flow conditions (EVM stack overflow) that are expected to be seen in
// normal
// operations.
@Override
public synchronized Throwable fillInStackTrace() {
return this;
}
}

/**
* Instantiates a new Fixed stack.
*
Expand Down
253 changes: 253 additions & 0 deletions evm/src/main/java/org/hyperledger/besu/evm/internal/FlexStack.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,253 @@
/*
* Copyright contributors to Hyperledger Besu
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
*/
package org.hyperledger.besu.evm.internal;

import static com.google.common.base.Preconditions.checkArgument;

import java.lang.reflect.Array;
import java.util.Arrays;
import java.util.Objects;

/**
* An operand stack for the Ethereum Virtual machine (EVM). The stack grows 32 entries at a time if
* it expands past the top of the allocated stack, up to maxSize.
*
* <p>The operand stack is responsible for storing the current operands that the EVM can execute. It
* is assumed to have a fixed size.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* is assumed to have a fixed size.
* is not assumed to have a fixed size.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed with slightly different verbiage.

*
* @param <T> the type parameter
*/
public class FlexStack<T> {

private static final int INCREMENT = 32;

private T[] entries;

private final int maxSize;
private int currentCapacity;

private int top;

/**
* Instantiates a new Fixed stack.
shemnon marked this conversation as resolved.
Show resolved Hide resolved
*
* @param maxSize the max size
* @param klass the klass
*/
@SuppressWarnings("unchecked")
public FlexStack(final int maxSize, final Class<T> klass) {
checkArgument(maxSize >= 0, "max size must be non-negative");
Copy link
Contributor

Choose a reason for hiding this comment

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

does it make sense to allow zero?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thats been in the codebase since before I started working in it. I can adjust it.


this.currentCapacity = Math.min(INCREMENT, maxSize);
this.entries = (T[]) Array.newInstance(klass, currentCapacity);
this.maxSize = maxSize;
this.top = -1;
}

/**
* Get operand.
*
* @param offset the offset
* @return the operand
*/
public T get(final int offset) {
if (offset < 0 || offset >= size()) {
throw new UnderflowException();
}

return entries[top - offset];
}

/**
* Pop operand.
*
* @return the operand
*/
public T pop() {
if (top < 0) {
throw new UnderflowException();
}

final T removed = entries[top];
entries[top--] = null;
return removed;
}

/**
* Peek and return type T.
*
* @return the T entry
*/
public T peek() {
if (top < 0) {
return null;
} else {
return entries[top];
}
}

/**
* Pops the specified number of operands from the stack.
*
* @param items the number of operands to pop off the stack
* @throws IllegalArgumentException if the items to pop is negative.
* @throws UnderflowException when the items to pop is greater than {@link #size()}
*/
public void bulkPop(final int items) {
checkArgument(items > 0, "number of items to pop must be greater than 0");
if (items > size()) {
throw new UnderflowException();
}

Arrays.fill(entries, top - items + 1, top + 1, null);
top -= items;
}

/**
* Trims the "middle" section of items out of the stack. Items below the cutpoint remains, and of
* the items above only the itemsToKeep items remain. All items in the middle are removed.
*
* @param cutPoint Point at which to start removing items
* @param itemsToKeep itemsToKeep Number of items on top to place at the cutPoint
* @throws IllegalArgumentException if the cutPoint or items to keep is negative.
* @throws UnderflowException If there are less than itemsToKeep above the cutPoint
*/
public void preserveTop(final int cutPoint, final int itemsToKeep) {
checkArgument(cutPoint >= 0, "cutPoint must be positive");
checkArgument(itemsToKeep >= 0, "itemsToKeep must be positive");
if (itemsToKeep == 0) {
if (cutPoint < size()) {
bulkPop(top - cutPoint);
}
} else {
int targetSize = cutPoint + itemsToKeep;
int currentSize = size();
if (targetSize > currentSize) {
throw new UnderflowException();
} else if (targetSize < currentSize) {
System.arraycopy(entries, currentSize - itemsToKeep, entries, cutPoint, itemsToKeep);
Arrays.fill(entries, targetSize, currentSize, null);
top = targetSize - 1;
}
}
}

@SuppressWarnings("unchecked")
private void expandEntries(final int nextSize) {
var nextEntries = (T[]) Array.newInstance(entries.getClass().getComponentType(), nextSize);
System.arraycopy(entries, 0, nextEntries, 0, currentCapacity);
entries = nextEntries;
currentCapacity = nextSize;
}

/**
* Push operand.
*
* @param operand the operand
*/
public void push(final T operand) {
final int nextTop = top + 1;
if (nextTop >= maxSize) {
throw new OverflowException();
}
if (nextTop >= currentCapacity) {
expandEntries(Math.min(currentCapacity + INCREMENT, maxSize));
}
entries[nextTop] = operand;
top = nextTop;
}

/**
* Set operand.
*
* @param offset the offset
* @param operand the operand
*/
public void set(final int offset, final T operand) {
if (offset < 0) {
throw new UnderflowException();
} else if (offset > top) {
throw new OverflowException();
}

entries[top - offset] = operand;
}

/**
* Size of entries.
*
* @return the size
*/
public int size() {
return top + 1;
}

@Override
public String toString() {
final StringBuilder builder = new StringBuilder();
for (int i = 0; i < top; ++i) {
builder.append(String.format("%n0x%04X ", i)).append(entries[i]);
}
return builder.toString();
}

@Override
public int hashCode() {
int result = 1;

for (int i = 0; i < currentCapacity; i++) {
result = 31 * result + (entries[i] == null ? 0 : entries[i].hashCode());
}

return result;
}

@SuppressWarnings("unchecked")
@Override
public boolean equals(final Object other) {
if (!(other instanceof FlexStack)) {
return false;
}

final FlexStack<T> that = (FlexStack<T>) other;
if (this.currentCapacity != that.currentCapacity) {
return false;
}
for (int i = 0; i < currentCapacity; i++) {
if (!Objects.deepEquals(this.entries[i], that.entries[i])) {
return false;
}
}
return true;
}

/**
* Is stack full.
*
* @return the boolean
*/
public boolean isFull() {
return top + 1 >= maxSize;
}

/**
* Is stack empty.
*
* @return the boolean
*/
public boolean isEmpty() {
return top < 0;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import org.apache.tuweni.bytes.Bytes;

/** The Operand stack. */
public class OperandStack extends FixedStack<Bytes> {
public class OperandStack extends FlexStack<Bytes> {

/**
* Instantiates a new Operand stack.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright contributors to Hyperledger Besu
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
*/
package org.hyperledger.besu.evm.internal;

/**
* Overflow exception for {@link FixedStack} and {@link FlexStack}. The main need for a separate
* class is to remove the stack trace generation as the exception is not used to signal a debuggable
* failure but instead an expected edge case the EVM should handle.
*/
public class OverflowException extends RuntimeException {

/**
* Overload the stack trace fill in so no stack is filled in. This is done for performance reasons
* as this exception signals an expected corner case not a debuggable failure.
*
* @return the exception, with no stack trace filled in.
*/
@Override
public synchronized Throwable fillInStackTrace() {
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import java.util.Objects;

/** The type Return stack. */
public class ReturnStack extends FixedStack<ReturnStack.ReturnStackItem> {
public class ReturnStack extends FlexStack<ReturnStack.ReturnStackItem> {

/** The type Return stack item. */
// Java17 convert to record
Expand Down
Loading
Loading