remove makeNotifierFromAsyncIterable from vat-timer #5709

turadg · 2022-07-01T18:08:16Z

What is the Problem Being Solved?

It's a "hot notifier" in the updater is driving the changes instead of the client. In a cold notifier the client asks for the next update and the notifier figures out what to do. Is doesn't any activity at all until the client calls next() again.

Description of the Design

Most of the uses of makeNotifierFromAsyncIterable take subscriptions so they'll be converted in #5695 to use the new function for those cases.

These two don't:

agoric-sdk/packages/SwingSet/src/vats/timer/vat-timer.js

Lines 66 to 73 in cafa284

    
           const iterable = makeTimedIterable( 
        
             timerService.delay, 
        
             timerService.getCurrentTimestamp, 
        
             baseTime, 
        
             interval, 
        
           ); 
        
           const notifier = makeNotifierFromAsyncIterable(iterable);

agoric-sdk/packages/zoe/tools/fakePriceAuthority.js

Lines 233 to 256 in cafa284

    
           async function* generateQuotes(amountIn, brandOut) { 
        
             let record = await ticker.getUpdateSince(); 
        
             while (record.updateCount) { 
        
               // eslint-disable-next-line no-await-in-loop 
        
               const { value: timestamp } = record; // = await E(timer).getCurrentTimestamp(); 
        
               yield priceInQuote(amountIn, brandOut, timestamp); 
        
               // eslint-disable-next-line no-await-in-loop 
        
               record = await ticker.getUpdateSince(record.updateCount); 
        
             } 
        
           } 
        
           /** @type {PriceAuthority} */ 
        
           const priceAuthority = Far('fake price authority', { 
        
             getQuoteIssuer: (brandIn, brandOut) => { 
        
               assertBrands(brandIn, brandOut); 
        
               return quoteIssuer; 
        
             }, 
        
             getTimerService: (brandIn, brandOut) => { 
        
               assertBrands(brandIn, brandOut); 
        
               return timer; 
        
             }, 
        
             makeQuoteNotifier: async (amountIn, brandOut) => { 
        
               assertBrands(amountIn.brand, brandOut); 
        
               return makeNotifierFromAsyncIterable(generateQuotes(amountIn, brandOut));

@michaelfig suspects the latter may solve a leak so tagging this with 'performance'.

Security Considerations

--

Test Plan

--

The text was updated successfully, but these errors were encountered:

warner · 2022-07-13T23:09:56Z

#5763 now tracks the zoe portion, this ticket now only tracks the swingset portion.

warner · 2022-07-14T08:19:49Z

I'll handle this as part of the #5668 rewrite of vat-timer/device-timer

vat-timer is now fully virtualized, durablized, and upgradeable. RAM usage should be O(N) in the number of: * pending Promise wakeups (`wakeAt`, `delay`) * active Notifier promises (`makeNotifier`) * active Iterator promises (`makeNotifier()[Symbol.asyncIterator]`) Pending promises will be disconnected (rejected) during upgrade, as usual. All handlers and Promises will fire with the most recent timestamp available, which (under load) may be somewhat later than the scheduled wakeup time. However Notifiers will always report a scheduled time (some multiple of the interval). The opaque `updateCount` used in Notifier updates is now a time value, not a counter, so user tests should refrain from asserting sequentiality. Asking for a wakeup in the past or present will fire immediately. Most API calls will accept an arbitrary Far object as a CancelToken, which can be used to cancel the wakeup/repeater. `makeRepeater` is the exception. This does not change the device-timer API or implementation, however vat-timer now only uses a single device-side wakeup, and only exposes a single handler object, to minimize the memory usage and object retention by the device (since devices do not participate in GC). This introduces a `Clock` which can return time values without also providing scheduling authority, and a `TimerBrand` which can validate time values without providing clock or scheduling authority. `packages/SwingSet/tools/manual-timer.js` offers a manually-driven timer service, which can help with unit tests. closes #5668 closes #5709 closes #4282 refs #4286 closes #4296 closes #5616 closes #5709 refs #5798

vat-timer is now fully virtualized, durablized, and upgradeable. RAM usage should be O(N) in the number of: * pending Promise wakeups (`wakeAt`, `delay`) * active Notifier promises (`makeNotifier`) * active Iterator promises (`makeNotifier()[Symbol.asyncIterator]`) Pending promises will be disconnected (rejected) during upgrade, as usual. All handlers and Promises will fire with the most recent timestamp available, which (under load) may be somewhat later than the scheduled wakeup time. Until cancellation, Notifiers will always report a scheduled time (i.e. `start` plus some multiple of the interval). The opaque `updateCount` used in Notifier updates is a counter starting from 1n. When a Notifier is cancelled, the final/"finish" value is the timestamp of cancellation, which may or may not be a multiple of the interval (and might be a duplicate of the last non-final value). Once in the cancelled state, `getUpdateSince(anything)` yields `{ value: cancellationTimestamp, updateCount: undefined }`, and the corresponding `iterator.next()` resolves to `{ value: cancellationTimestamp, done: true }`. Neither will ever reject their Promises (except due to upgrade). Asking for a wakeup in the past or present will fire immediately. Most API calls will accept an arbitrary Far object as a CancelToken, which can be used to cancel the wakeup/repeater. `makeRepeater` is the exception. This does not change the device-timer API or implementation, however vat-timer now only uses a single device-side wakeup, and only exposes a single handler object, to minimize the memory usage and object retention by the device (since devices do not participate in GC). This introduces a `Clock` which can return time values without also providing scheduling authority, and a `TimerBrand` which can validate time values without providing clock or scheduling authority. Timestamps are not yet Branded, but the scaffolding is in place. `packages/SwingSet/tools/manual-timer.js` offers a manually-driven timer service, which can help with unit tests. closes #5668 closes #5709 closes #4282 refs #4286 closes #4296 closes #5616 closes #5709 refs #5798

vat-timer is now fully virtualized, durablized, and upgradeable. RAM usage should be O(N) in the number of: * pending Promise wakeups (`wakeAt`, `delay`) * active Notifier promises (`makeNotifier`) * active Iterator promises (`makeNotifier()[Symbol.asyncIterator]`) Pending promises will be disconnected (rejected) during upgrade, as usual. All handlers and Promises will fire with the most recent timestamp available, which (under load) may be somewhat later than the scheduled wakeup time. Until cancellation, Notifiers will always report a scheduled time (i.e. `start` plus some multiple of the interval). The opaque `updateCount` used in Notifier updates is a counter starting from 1n. When a Notifier is cancelled, the final/"finish" value is the timestamp of cancellation, which may or may not be a multiple of the interval (and might be a duplicate of the last non-final value). Once in the cancelled state, `getUpdateSince(anything)` yields `{ value: cancellationTimestamp, updateCount: undefined }`, and the corresponding `iterator.next()` resolves to `{ value: cancellationTimestamp, done: true }`. Neither will ever reject their Promises (except due to upgrade). Asking for a wakeup in the past or present will fire immediately. Most API calls will accept an arbitrary Far object as a CancelToken, which can be used to cancel the wakeup/repeater. `makeRepeater` is the exception. This does not change the device-timer API or implementation, however vat-timer now only uses a single device-side wakeup, and only exposes a single handler object, to minimize the memory usage and object retention by the device (since devices do not participate in GC). This introduces a `Clock` which can return time values without also providing scheduling authority, and a `TimerBrand` which can validate time values without providing clock or scheduling authority. Timestamps are not yet Branded, but the scaffolding is in place. `packages/SwingSet/tools/manual-timer.js` offers a manually-driven timer service, which can help with unit tests. closes #4282 refs #4286 closes #4296 closes #5616 closes #5668 closes #5709 refs #5798

warner · 2022-08-19T07:13:49Z

The new vat-timer implements the Notifier API directly, without using makeNotifierFromAsyncIterable. It implements the Iterable API directly too. The memory usage should be proportional to the number of unresolved Promises fetched by clients with getUpdateSince() and next(), respectively.

turadg added enhancement New feature or request performance Performance related issues notifier technical-debt labels Jul 1, 2022

turadg added this to the Mainnet 1 milestone Jul 1, 2022

turadg mentioned this issue Jul 6, 2022

Bank vat has a memory leak #5671

Closed

Tartuffo added the SwingSet package: SwingSet label Jul 11, 2022

warner added the Zoe package: Zoe label Jul 13, 2022

warner mentioned this issue Jul 13, 2022

remove makeNotifierFromAsyncIterable from zoe/tools/fakePriceAuthority.js #5763

Open

warner self-assigned this Jul 13, 2022

warner removed the Zoe package: Zoe label Jul 13, 2022

warner changed the title ~~remove makeNotifierFromAsyncIterable~~ remove makeNotifierFromAsyncIterable from vat-timer Jul 14, 2022

Tartuffo removed this from the Mainnet 1 milestone Jul 19, 2022

Tartuffo added possible-post-launch and removed possible-post-launch labels Jul 19, 2022

Tartuffo added this to the Mainnet 1 RC0 milestone Aug 10, 2022

warner mentioned this issue Aug 16, 2022

overhaul vat-timer: virtualized, durable, upgradable, Clock, TimerBrand #5847

Merged

mergify bot closed this as completed in 34cff5f Aug 19, 2022

mergify bot closed this as completed in #5847 Aug 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remove makeNotifierFromAsyncIterable from vat-timer #5709

remove makeNotifierFromAsyncIterable from vat-timer #5709

turadg commented Jul 1, 2022

warner commented Jul 13, 2022

warner commented Jul 14, 2022

warner commented Aug 19, 2022

remove makeNotifierFromAsyncIterable from vat-timer #5709

remove makeNotifierFromAsyncIterable from vat-timer #5709

Comments

turadg commented Jul 1, 2022

What is the Problem Being Solved?

Description of the Design

Security Considerations

Test Plan

warner commented Jul 13, 2022

warner commented Jul 14, 2022

warner commented Aug 19, 2022