[WIP] [PoC] Trying to change object layout

[osa1]

No description provided.

[nomeata]

I would suggest to introduce a Tagged.add_static so that you can write

    Tagged.add_static env Closure [
      I32 (E.add_fun_ptr env fi);
      I32 0l
    ]

here and similar in other places. That’s a quick and worthwhile refactoring you can quickly do in a PR agaist master, and will simplify work like this.

I.e.: Clean up and refactor the code (with self-contained simple PRs) until this PR works :-)

Unless this goes in a very different direction than what you envision in the long run, of course.

[nomeata]

A bit hard to debug, I guess I’d have to go through all lines that you didn’t change and check these – hard to do that with a PR…

[nomeata]

I don’t see a clear connection between the failing test cases you list yet. Maybe text and ropes?

[nomeata]

Not updated (can’t comment on unchanged lines)

        TAG_BITS64 => Words(3),

[nomeata]

Also

        TAG_BITS32 => Words(2),

But most importantly, it seems you didn’t update rts/text.c

[osa1]

Currently debugging a bug in object field reads. Reproducer:

class Foo() {
    public var bar : Bool = true;
};

Prim.debugPrint(debug_show (Foo()));
Prim.printObject(Foo());

Output:

{bar = false}
0x17064: <Object size=0x1 hash_ptr=0x10063 field=[0x17077 (indirectee=0x17078: <ObjInd field=0x2>)]>

The first line is incorrect (it should print "true" not "false"), but if I peek at the object internals it looks right. So I think there's a bug when reading the field.

[nomeata]

Could also be a confusion between shifted and unshifted bool somewhere, possibly in printing. Does if Foo().bar behave correctly or wrongly?

[osa1]

Does if Foo().bar behave correctly or wrongly?

That also doesn't work as expected. This prints false:

import Prim "mo:prim";

class Foo() {
    public var bar : Bool = true;
};

let foo = Foo ();

if (foo.bar) {
    Prim.debugPrint "--- true";
} else {
    Prim.debugPrint "--- false";
}

[osa1]

Found it .. fix coming.

[osa1]

Amusingly the last commit fixed dozens of tests that were working fine previously. I think the commit is correct and revealed bugs in other places.

[osa1]

Hard to parse run.sh output, but I think literally every "run-drun" test is broken now. It's probably just one bug in a very commonly executed code. "run" tests are running fine now.

[osa1]

I think remaining of the bugs are in serialization and deserialization code.

[osa1]

Sigh.. this is really tiresome, especially without a debugger. @nomeata do you know the locations where we're allocating heap objects in serialization and deserialization code?

[nomeata]

They are all in the Serialization module, in deserialize_go. (I don't think we allocate when serializing, besides maybe one big blob to store the data – maybe that is the problem). Most should use the more abstract interfaces from a bit above, but maybe not all.

[nomeata]

The PR description is not up to date, is it? It seems that mut-field-bound.wasm-run nested-lexpr.wasm-run objects5.wasm-run are passing here.

[nomeata]

Minimal example of failing code:

actor a {
  public func bar() : async (){
  };
  public func go(){
    await bar();
  };
};
a.go(); //OR-CALL ingress go "DIDL\x00\x00"

produces

+Trap: EvalTrapError :0.1 "uninitialized element 144"

(but oddly not on drun… weird). Unlikely to be in serialization (it’s all () here), but maybe in the output of async-await and/or mutable data.

[nomeata]

I might have found it:

--- a/src/codegen/compile.ml
+++ b/src/codegen/compile.ml
@@ -5577,7 +5577,7 @@ module AllocHow = struct
       let (ae1, i) = VarEnv.add_direct_local env ae name in
       (ae1, G.nop)
     | StoreHeap ->
-      let (ae1, i) = VarEnv.add_local_with_offset env ae name 1l in
+      let (ae1, i) = VarEnv.add_local_with_offset env ae name 2l in
       let alloc_code =
         Tagged.obj env Tagged.MutBox [ compile_unboxed_zero ] ^^
         G.i (LocalSet (nr i)) in
@@ -6979,7 +6979,7 @@ and compile_exp (env : E.t) ae exp =
       code1 ^^ set_i ^^ orTrap env code2 ^^ get_j
   (* Async-wait lowering support features *)
   | DeclareE (name, _, e) ->
-    let (ae1, i) = VarEnv.add_local_with_offset env ae name 1l in
+    let (ae1, i) = VarEnv.add_local_with_offset env ae name 2l in
     let sr, code = compile_exp env ae1 e in
     sr,
     Tagged.obj env Tagged.MutBox [ compile_unboxed_zero ] ^^

now more stuff works.

I am unsure about

  let get_val_ptr env ae var = match VarEnv.lookup_var ae var with
    | Some (HeapInd (i, 2l)) -> G.i (LocalGet (nr i))
    | Some (HeapStatic _) -> assert false (* we never do this on the toplevel *)
    | _  -> field_box env (get_val_vanilla env ae var)

now, and will have to think about it, but not today.

[nomeata]

This one looks fun! Double-Candid-Encoding somehow?

--- caller.drun-run (expected)
+++ caller.drun-run (actual)
@@ -6,7 +6,7 @@
 ingress Completed: Reply: 0x4449444c0000
 Ok: Reply: 0x4449444c0000
 ingress Completed: Reply: 0x4449444c000168010a0100000000000000fe01
-Ok: Reply: 0x4449444c0001680112010000000000000000000000000000000001
+Ok: Reply: 0x4449444c00016801134449444c000168010a0100000000000000fe01
 Ok: Reply: 0x4449444c000179edff70f1
 Ok: Reply: 0x4449444c00017d0a
 Ok: Reply: 0x4449444c016e7b01000101

Facts:

• The bug appears if I run only c1 and c8
• The bug disappears if I remove the ctxt argument of c1. So likely related to allocation of GC or either objects or blobs (in particular the caller blob)
• The bug disappears if Dfinity.caller just returns a static blob, and does not call ic0.msg_caller_copy…

[dfinity-ci]

In terms of gas, 3 tests regressed and the mean change is +1.9%.
In terms of size, 3 tests regressed and the mean change is +0.5%.

[nomeata]

This was a useful experiment, and it is still green … still, is this worth keeping, or should we closeit?

[osa1]

I think we can close it. All hard-coded offset/size values found during development of this are replaced with size_ofs calls (in the RTS) or variables (in the backend), so if we need something like this in the future hopefully it won't be too difficult to do.