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

EEI: add metering of memory #75

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
6 changes: 4 additions & 2 deletions eth_interface.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@ We also define the following WebAssembly data types:

## useGas

Subtracts an amount to the gas counter
Reduces the gas left counter by an amount.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the "gas left counter" kept outside of the WASM?

Copy link
Member Author

@axic axic Jan 22, 2018

Choose a reason for hiding this comment

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

It is kept in the VM.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see here one problem, the takeGas function does not provide any reference to the current VM execution context. How would a VM handle this in concurrent environment?

Copy link
Member Author

Choose a reason for hiding this comment

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

There are no references in any of the methods. Each contract has its own instance, it is an implementation detail in VM how to add the context to the methods.

Copy link
Collaborator

Choose a reason for hiding this comment

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

How is Hera doing it?

Copy link
Member Author

Choose a reason for hiding this comment

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

There's an instance of the methods for each execution (follows the sample way binaryen gives). It can be optimised though, but shouldn't affect the interface.


It should also charge for memory cost by multiplying `memory_pages * memory_cost * amount`, where `memory_cost` is defined by the gas schedule.
Copy link
Member Author

Choose a reason for hiding this comment

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

Actually this could be implemented in the metering process by

(call $useGas
  (i64.mul
    (i64.const <instruction gas>)
    (i64.mul (i64.const <memory cost per page>) (current_memory))))

Though the two muls could be precomputed into one. I'd say this wouldn't have any lower overhead then doing it in the interface, but does remove possibility to adjust memory cost after metering.

Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. Shouldn't it just charge for "added" memory pages instead of charging for all of them each time?
  2. In the comment you have <instruction gas> * <memory cost>. Shouldn't it be +?

Copy link
Member Author

Choose a reason for hiding this comment

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

The idea here is to charge for memory pages for the given time those are alive. The time dimension is metered by instruction gas (eg how many cycles it takes). In every block the entire available memory is charged for the duration it is required.

This is an idea @wanderer and I discussed and removes the need for the quadratic memory cost rule.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I noticed it will be some-way quadratic.


**Parameters**

- `amount` **i64** the amount to subtract to the gas counter
- `amount` **i64** the amount to reduce the gas left counter with
Copy link
Member

Choose a reason for hiding this comment

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

s/with/by/

or

"the amount by which to reduce the remaining gas counter"


**Returns**

Expand Down