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

virtual object manager must retain strongref to Remotables in virtualized data #3132

Closed
warner opened this issue May 19, 2021 · 0 comments · Fixed by #3153
Closed

virtual object manager must retain strongref to Remotables in virtualized data #3132

warner opened this issue May 19, 2021 · 0 comments · Fixed by #3153
Assignees
Labels
enhancement New feature or request SwingSet package: SwingSet

Comments

@warner
Copy link
Member

warner commented May 19, 2021

What is the Problem Being Solved?

If a Remotable is put into the property of a virtual object, or into the value of a virtualized store (like the VOM's makeWeakStore()), userspace not keep a separate strong reference to that Remotable, but it must still be retrievable when the virtual object state or virtualized value is accessed. The offline data will remember the Remoteable's vref, but cannot hold the Remotable itself (since they cannot be regenerated on demand, unlike Presences and Representatives).

To support this, the virtual object manager must maintain a strong reference to any Remotable which appears in this virtualized data. For now, we should simply maintain a Set, and add the Remoteable as soon as we realize it's being included.

For virtual objects, this will happen just after the state property has been written and serialized. We need to scan all the resulting slots, identify the ones that are non-virtual exports (o+NN, not o-NN or o+NN/YY), use slotToVal(vref).deref() to get the Remotable, and add it to the Set. For virtualized values, we do the same in .set(), just after serializing the value.

We can get away with append-only for now because we only have a single customer so far, and it only deposits a single Remotable. ERTP's Payment is a virtual object whose only state property is a Brand (which is a Remotable), and the Payment is used in as the key in a makeWeakStore() to store an Amount (which is a BigInt and a Brand). There is one Brand per token type.

Later, we'll need to improve the VOM to perform refcounting on these Remotables, and release the strong reference when it is no longer referenced by anything. Since the world that needs this performance improvement is probably also one in which we're dealing with a lot of Remotables, we'll need to keep those refcounts on disk too.

(A separate question for @erights and his virtual-store work is whether Remotables are eligible for inclusion in this virtualized data. But, our current implementations accept them, so they must be handled properly for now).

Description of the Design

  • change VOM state setter to scan slots, identify o+NN vrefs, use slotToVal(vref).deref() to get the Remotable, add to a Set, never remove it
  • change makeWeakStore() .set method to do the same
  • add ticket to track the future work of refcounting and releasing Remotables when they can no longer be retrieved through virtualized data: implement refcounts for virtualized data, drop unreachable Remotables #3149

Test Plan

When #2660 lands, I'll have unit test examples which provoke GC() in the middle of a test. I think that will give us a way to write a unit test for this:

  • let rem = Far(..)
  • let wr = new WeakRef(rem) to track it
  • pick a vref, forge a Presence
  • make a WeakStore, do .set(presence, rem)
  • rem = undefined to drop rem
  • force GC()
  • do wr.deref() !== undefined to make sure it's still alive

We should first run this test without the Set strongref and make sure it fails, then add the Set and make sure it succeeds.

And then we'll need to do the same with the .state of a virtual object, which is a bit more tricky because the LRU cache might keep it alive independently of the strongref Set.

@warner warner added enhancement New feature or request SwingSet package: SwingSet labels May 19, 2021
@warner warner assigned warner and unassigned FUDCo May 20, 2021
warner added a commit that referenced this issue May 21, 2021
Userspace might store a locally-created Remotable (e.g. `Far('iface',
{methods..})` in the `state` of a virtual object, or somewhere in the value
of a vref-keyed `makeWeakStore()` entry. In either case, the data is
virtualized: serialized and written to disk. This serialized form obviously
cannot keep the Remotable JS `Object` alive directly, however userspace
reasonably expects to get the Remotable back if it reads the `state` or does
a `.get` on the store.

To ensure the Remotable can be looked up from the serialized vref, the
virtual object manager must retain a strong reference to the original
Remotable for as long as its vref is present anywhere in the virtualized
data.

For now, we simply add the Remotable to a strong Set the first time it is
added, and we never remove it. This is safe, but conservative.

To do better (and eventually release the Remotable), we'll need to add some
form of refcount to each vref. When the refcount of the Remotable's vref
drops to zero, the VOM can drop its strong reference to the Remotable.

closes #3132
refs #3106
warner added a commit that referenced this issue May 21, 2021
Userspace might store a locally-created Remotable (e.g. `Far('iface',
{methods..})` in the `state` of a virtual object, or somewhere in the value
of a vref-keyed `makeWeakStore()` entry. In either case, the data is
virtualized: serialized and written to disk. This serialized form obviously
cannot keep the Remotable JS `Object` alive directly, however userspace
reasonably expects to get the Remotable back if it reads the `state` or does
a `.get` on the store.

To ensure the Remotable can be looked up from the serialized vref, the
virtual object manager must retain a strong reference to the original
Remotable for as long as its vref is present anywhere in the virtualized
data.

For now, we simply add the Remotable to a strong Set the first time it is
added, and we never remove it. This is safe, but conservative.

To do better (and eventually release the Remotable), we'll need to add some
form of refcount to each vref. When the refcount of the Remotable's vref
drops to zero, the VOM can drop its strong reference to the Remotable.

closes #3132
refs #3106
warner added a commit that referenced this issue May 21, 2021
Userspace might store a locally-created Remotable (e.g. `Far('iface',
{methods..})` in the `state` of a virtual object, or somewhere in the value
of a vref-keyed `makeWeakStore()` entry. In either case, the data is
virtualized: serialized and written to disk. This serialized form obviously
cannot keep the Remotable JS `Object` alive directly, however userspace
reasonably expects to get the Remotable back if it reads the `state` or does
a `.get` on the store.

To ensure the Remotable can be looked up from the serialized vref, the
virtual object manager must retain a strong reference to the original
Remotable for as long as its vref is present anywhere in the virtualized
data.

For now, we simply add the Remotable to a strong Set the first time it is
added, and we never remove it. This is safe, but conservative.

To do better (and eventually release the Remotable), we'll need to add some
form of refcount to each vref. When the refcount of the Remotable's vref
drops to zero, the VOM can drop its strong reference to the Remotable.

closes #3132
refs #3106
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request SwingSet package: SwingSet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants