Uncaught Error: Element type must be nullable if array is holey in ~lib/array.ts:118:40

[kuvkir]

I'm running into a weird issue when passing a relatively large array of objects from JS into wasm.

on the AssemblyScript side I declare the following class and a function

export class TestEntity {
  readonly id: entity.EntityId = 0;
}

export function testEntity(arr: TestEntity[]): TestEntity[] {
  for (let i = 0; i < arr.length; i++) {
    const e = arr[i];
    console.log(`#i ${i} ${e.id}`);
  }
  return arr;
}

on the JS side I run the following code, which basically creates an array of 1125 instances of TestEntity with the corresponding id (using lodash times) and passes it down to the wasm code.

      const arr = _.times(1125, id => ({ id }));
      gamelib.testEntity(arr);

I'm using the generated host bindings to pass the data.

When I use the relatively small array (a few dozens of elements) all works well. However, with large enough number of elements (e.g. 1125 in the example above), I am running into the following error message:

gamelib.debug.js:12 Uncaught Error: Element type must be nullable if array is holey in ~lib/array.ts:118:40
    at gamelib.debug.js:12:17
    at abort (gamelib.debug.js:13:10)
    at ~lib/array/Array<assembly/PlainObject/TestEntity>#__get (PlainObject.ts:128:39)
    at assembly/PlainObject/testEntity (PlainObject.ts:60:19)
    at export:assembly/PlainObject/testEntity (array.ts:121:5)
    at Object.testEntity (gamelib.debug.js:198:111)

The stack trace shows the error occurs in the array.ts [] operator ( @operator("[]") private __get(index: i32): T ).
From the error message, the wasm code complains that the array is holey, while it is definitely not. It only contains the instances of TestEntity class.

[kuvkir]

For the sake of debugging, I modified the generated host binding to look directly into the passed array memory layout. And all the elements are still there. There are no holes (nulls) in it.

testEntity(arr) {
  // assembly/PlainObject/testEntity(~lib/array/Array<assembly/PlainObject/TestEntity>) => ~lib/array/Array<assembly/PlainObject/TestEntity>

  let m = new Uint32Array(memory.buffer);
  console.log(`m.length`, m.length);
  let im = new Int32Array(memory.buffer);
  arr =
    __lowerArray(
      (pointer, value) => {
        new Uint32Array(memory.buffer)[pointer >>> 2] = __lowerRecord71(value) || __notnull();
      },
      72,
      2,
      arr
    ) || __notnull();

  const pointer = arr;

  m = new Uint32Array(memory.buffer);
  im = new Int32Array(memory.buffer);

  const dataStart = m[(pointer + 4) >>> 2];

  const memoryU32 = new Uint32Array(memory.buffer);
  console.log(`pointer`, arr, (pointer + 4) >>> 2, memoryU32[(pointer + 4) >>> 2]);
  console.log(`dataStart`, dataStart);
 // not calling the wasm code, just looking into the array
  // const result = exports.testEntity(arr) >>> 0;
  const result = pointer;
  const aaaa = __liftArray(
    pointer => {
      console.log(`-- pointer`, pointer, m[pointer >>> 2], im[m[pointer >>> 2] >>> 2]);
      return __liftRecord71(new Uint32Array(memory.buffer)[pointer >>> 2]);
    },
    2,
    result
  );
  console.log(`result`, result);
  console.log(aaaa);
  return aaaa;
},

[dcodeIO]

Hmm. Does the error go away when increasing --initialMemory? Just a guess so far, as this could have something to do with growing memory during lowering.

[kuvkir]

@dcodeIO thanks, I just did that and the error has gone away after setting --initialMemory 2

After carefully looking into my debug output (without the --initialMemory flag), there is indeed a hole in the array, at 64kb (1 page) boundary.

-- pointer 40952 65504 718
-- pointer 40956 0 0
-- pointer 40960 65568 720
-- pointer 40964 65600 721

That seems to be a bug in the generated bindings?

[dcodeIO]

Interesting, indeed looks like a problem with growing somewhere in the middle of lowering an array's elements. Looking at some bindings code currently, but I'm not yet seeing what could cause this. Perhaps, one strategy could be to instrument the bindings code, and see what values it actually stores to the array, hmm. (assuming the current log comes from within Wasm?)

[kuvkir]

The problem seems to depend on the order of memory buffer creation / memory growing inside the __lowerRecord code
the original generated code is somewhat tricky to make sense of. The assumption is probably is that the left part should be evaluated after the right one, so the memory.buffer has already grown, which doesn't seems to be the case here...

new Uint32Array(memory.buffer)[pointer >>> 2] = __lowerRecord71(value) || __notnull();

I modified the testEntity function from the bindings code as follows:

testEntity(arr) {
  // assembly/PlainObject/testEntity(~lib/array/Array<assembly/PlainObject/TestEntity>) => ~lib/array/Array<assembly/PlainObject/TestEntity>
  arr =
    __lowerArray(
      (pointer, value) => {
        let m_old = new Uint32Array(memory.buffer);
        // this is the original line
        // new Uint32Array(memory.buffer)[pointer >>> 2] = __lowerRecord71(value) || __notnull();
        // if m_old is evaluated before __lowerRecord71, the value is undefined and m_old length is 0
        m_old[pointer >>> 2] = __lowerRecord71(value) || __notnull();

        // if m_new is evaluated after __lowerRecord, the correct value is written into it
        // const val = __lowerRecord71(value) || __notnull();
        let m_new = new Uint32Array(memory.buffer);
        // m_new[pointer >>> 2] = val;
        console.log(`storing value ${value.id} `);
        console.log(`-- m_old: ${m_old[m_old[pointer >>> 2] >>> 2]} len ${m_old.length}`);
        console.log(`-- m_new: ${m_new[m_new[pointer >>> 2] >>> 2]} len ${m_new.length}`);
      },
      72,
      2,
      arr
    ) || __notnull();
  return __liftArray(
    pointer => __liftRecord71(new Uint32Array(memory.buffer)[pointer >>> 2]),
    2,
    exports.testEntity(arr) >>> 0
  );
},

Here is what I'm getting in the output

storing value 715 
-- m_old: 715 len 16384
-- m_new: 715 len 16384
 storing value 716 
-- m_old: undefined len 0
-- m_new: 0 len 32768
storing value 717 
-- m_old: 717 len 32768
-- m_new: 717 len 32768

So the correct value is not being written into a newly allocated memory, nor into the original memory buffer (due to having not enough size). If the correct evaluation order is enforced (__lowerRecord which allocates new memory, then Uint32 buffer creation) the code behaves as expected.

[dcodeIO]

Great analysis :) So I guess the bindings should instead do something akin to:

let t = __lowerRecord71(value) || __notnull();
new Uint32Array(memory.buffer)[pointer >>> 2] = t;

Another option might be to abstract the new Uint32Array(memory.buffer)[pointer >>> 2] into a function (say storeU32(pointer, value)), but then I'm not sure how well this optimizes.

[kuvkir]

Yes, that seems to work fine.

Worth noting that this sort of syntax (with new Uint32Array indexing as an l-value) is used all across the generated code in for all kinds of records, not just in the __lowerArray function.

For the time being, the --initialMemory flag does its job to avoid extra allocation.

Thanks for looking into it.