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

EEI: add metering of memory #75

wants to merge 1 commit into from

Conversation

axic
Copy link
Member

@axic axic commented Jan 21, 2018

No description provided.

Subtracts an amount to the gas counter
Reduces the gas left counter by an amount.

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.

Subtracts an amount to the gas counter
Reduces the gas left counter by an amount.

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
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 +?

@@ -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.

@axic
Copy link
Member Author

axic commented Jan 24, 2018

A relevant old issue: ewasm/wasm-metering-old#1

@poemm
Copy link
Contributor

poemm commented Jul 31, 2018

I think that useGas for memory should be injected in two places: i) at the beginning of the main function to charge for pre-allocated memory in the memory section, and ii) before each memory.grow Wasm opcode. The cost for each memory page should reflect zeroing-out a 64kB page. memory.grow should be more expensive since it is done at run-time.

To prevent resource exhaustion, we can have either i) a sentinel validation rule that the memory section must have a max e.g. 1024 memory pages, or ii) an algebraically or exponentially growing memory gas cost. The second option makes gas cost known only at run-time, which is undesirable.

Any other options or ideas?


**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"

@axic axic added the ready label Aug 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants