comms refcounts mix Number and BigInt

[warner]

Describe the bug

I haven't seen it cause problems yet, but I noticed src/vats/comms/state.js was mixing Numbers and BigInts in a way that might throw an error:

  function decrementRefCount(lref, tag, mode = 'data') {
    assert(referenceModes.includes(mode), `unknown reference mode ${mode}`);
    const { type } = parseLocalSlot(lref);
    if (type === 'promise') {
      let refCount = parseInt(store.get(`${lref}.refCount`), 10);
      assert(refCount > 0n, X`refCount underflow {lref} ${tag}`);
      refCount -= 1;
      // cdebug(`-- ${lref}  ${tag} ${refCount}`);
      store.set(`${lref}.refCount`, `${refCount}`);
      if (refCount === 0) {

I was going to add a Nat(refCount) to that store.set call, to catch negative-going refcounts on their way into the DB, when I noticed that:

• parseInt will return a Number
  • (in many other places, we use Nat(Number(store.get(key))), which would get us a BigInt, although to be honest I don't know what conversion we should use)
• the assert will compare a Number against a BigInt, which doesn't cause an error (but I don't know why)
• the -= 1 only works with a Number, and would cause TypeError against a BigInt
• the === 0 would return false if the refcount is 0n

So the code is functional, but a bit inconsistent.

We should choose one or the other, make a pass through this file, and make everything consistent. I'd like to add more Nat()s as assertions, which will probably push us towards using BigInt everywhere.

[FUDCo]

Using BigInts for refcounts seems very silly to me. We're paying the overhead for BigInts and getting no benefit in return. You'd have to turn more than all the mass of the solar system into computronium running a single swingset and then have every memory element point to the same object before you'd overflow a Number. Attempting to decrement a refcount below zero is a bug we already have to check for, so using a Nat to do it also seems like overkill.

[FUDCo]

This is another "should we really do this?" conversation we should have.

[erights]

Are bigints more expensive than numbers? Where?

[erights]

If expense isn't the decider, and our code already mixes the two representations, so status quo isn't a decider, bigint is still the semantically nicer basis. What's a fractional refcount?

While I agree that refcounts are unlikely to overflow 2**53, some uses of numbers-as-integers, like the bitwise ops, truncate them to 2**32, which is not safe from overflow. I doubt we have such code. But if it doesn't cost anything, using bigints simply avoids these hazards.

[FUDCo]

Are bigints more expensive than numbers? Where?

Computationally they are more expensive and in some cases substantially so (100x or more in some cases, though the really expensive operation is multiplication which we're mostly not doing). However, I seriously doubt that cost is significant in the contexts where we are using them, which are mostly simple counters. The big expense from my perspective is that they don't play nice with all the other APIs that expect numbers, which is an ongoing and irritating source of development friction. On the other hand, the benefit of having them in contexts like refcounts is zilch.

[erights]

Computationally they are more expensive and in some cases substantially so (100x or more in some cases, though the really expensive operation is multiplication which we're mostly not doing)

For numbers in the same range? That's odd.

On the other hand, the benefit of having them in contexts like refcounts is zilch.

but...

While I agree that refcounts are unlikely to overflow 253, some uses of numbers-as-integers, like the bitwise ops, truncate them to 232, which is not safe from overflow. I doubt we have such code. But if it doesn't cost anything, using bigints simply avoids these hazards.

Small but not zilch.

[FUDCo]

We actually do use bitwise ops in the stores implementation, but it's specifically to do with fiddling with the binary encoding of 64-bit IEEE floats!