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

Zoe metering #592

Merged
merged 10 commits into from
Feb 26, 2020
Merged

Zoe metering #592

merged 10 commits into from
Feb 26, 2020

Conversation

michaelfig
Copy link
Member

@michaelfig michaelfig commented Feb 23, 2020

Implements basic metering for Zoe contracts. Refs #516

Closes #546

This is a temporary measure until dynamic vats are metered and Zoe can spawn vats instead of evaluating all user code in one vat.

@katelynsills please review the Zoe changes.

@warner please review the additional SwingSet changes.

@michaelfig
Copy link
Member Author

Added spawner support while I am at it. PTAL.

Copy link
Contributor

@katelynsills katelynsills left a comment

Choose a reason for hiding this comment

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

The Zoe parts look great to me! I'd love to understand more about how the metering works.

@michaelfig
Copy link
Member Author

We can talk about it in person this week. 😄

@michaelfig
Copy link
Member Author

Closes #546

@warner
Copy link
Member

warner commented Feb 24, 2020

Notes to self about how this works (gleaned in person with Michael):

  • The one kernel/vat Realm is configured with a transformation function that will inject metering code iff a getGlobalMeter endowment is also present (the injected code decrements balances by calling getGlobalMeter(), so it can't work without it). Neither the kernel nor static vats are given this endowment, so they are unmetered.
  • The builtins (Array/RegExp/etc) are modified to decrement globalMeter, if present. The controller creates replaceGlobalMeter to set this global variable, and makes it available as an endowment to the kernel and to static vats. Dynamic contracts won't get access, since the code that evaluates them doesn't pass the endowment through.
  • The 'spawner' vat loads dynamic contract code at runtime. We aren't using dynamic vats yet, so these contracts are evaluated within the spawner vat, and it wants to distinguish its own cycles from the contract's cycles.
  • The 'spawner' vat imports makeMeter from transform-metering, and uses it to make meters for dynamic contract code. It creates a getGlobalMeter endowment (which returns one of these meters) for the evaluate call that loads the contract code; the presence of the endowment triggers the injection of metering code.
  • The PR hacks replaceGlobalMeter to take a second argument that registers a callback to be run (by the kernel) when the crank is complete. The spawner vat arranges for this to be called by the injected code, and the callback it registers will refill the contract's meter.
  • By refilling the meter at the end of each crank, we're only enforcing single-delivery bounds, rather than cumulative bounds.

Michael assures me that this complexity will go away soon:

  • once we're using dynamic vats, the spawner won't need to distinguish between its own cycles and those of a contract is has loaded
  • the SES2 switch should make something simpler (? I forget the details)

There remain a few questions about what the consequences of metering ought to be. When a meter underflows, computation is interrupted (by an exception that cannot be caught by the guest code) and the kernel regains control, knowing that the crank ended because of an underflow.

At that point, one option is to terminate the entire vat (the "one false move and you're dead" approach). This is certainly safer in terms of integrity, as there are no invariants that could be violated, but it's the worst in terms of liveness. All subsequent messages are rejected. We'd discard all of the state changes (c-list additions) made by the doomed crank. The only state changes that get committed would be deleting the doomed vat and popping the fatal message off the run-queue.

The second option is to treat the message delivery as failed, but allow subsequent messages to be delivered. The vat state observed by the subsequent message is from the middle of the failing crank: everything up until the uncatchable exception is still there. If we shut down and restart the swingset machine, that vat will follow the same path as before, underflow the meter just as before, and will be left in the same middle-of-the-crank state.

The main problem with this option is that the vat has no way to run a finally block to recover invariants after a potential underflow (if it could, it could probably escape the metering). This is doubly bad if the vat is running guest code on somebody else's behalf, where being able to clean up after the guest is more important.

We're currently implementing the second option. The kernel treats meter underflow the same way it treats any other vat error during message delivery: the result promise (if any) is rejected, and the kernel goes back to the run queue to process the next delivery.

Copy link
Member

@warner warner left a comment

Choose a reason for hiding this comment

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

We walked through this in person and it sounds good to land. I'm concerned about the complexity (and particularly the fact that this is pretty opaque and hard to understand without an in-person conversation), but it sounds like this will be improved soon.

@michaelfig
Copy link
Member Author

@dtribble made some comments that suggested important changes:

  1. fail-stop instead of best-effort: the meters should not be refilled once they are exhausted. Exhaustion is blamed on the contract code, not the messages sent to them, which encourages predictability and integrity over liveness.
  2. To implement the above point on spawner, the individual .spawn() calls should have their own meters, rather than a shared meter per .install().

Copy link
Member Author

@michaelfig michaelfig left a comment

Choose a reason for hiding this comment

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

Notes for rework.

@michaelfig michaelfig merged commit f755266 into master Feb 26, 2020
@michaelfig michaelfig deleted the 516-zoe-metering branch February 26, 2020 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gas with SES 1.0
3 participants