Capturing module bindings locally leads to grave performance hits

[ggreif]

The idiom of abbreviating module bindings locally

        let {attempt; equals; inRange} = M;

leads to inflated closure environments, and excessive closure unpacking/repacking (notably in async context).

The solution is probably to continue to treat such bindings as module variables, just with a new identifier.

[nomeata]

I think the solution is to make sure that https://github.com/dfinity-lab/motoko/blob/master/src/ir_passes/const.ml can handle them, so that they remain const static references. Because if that pass works as it should, destructing a module in a pattern should yield identical code than using M.attempt etc.

[kritzcreek]

I think the solution is to allow destructuring pattern matches in import statements, and making the const pass understand those.

So we can write:

import M "mo:matchers/Matchers";
import { attempt; equals; inRange } "mo:matchers/Matchers";

[nomeata]

Well, by the time we reach the const analysis, imports are plain lets with structured pattern. And I thought we support those already.

Can we have a minimal example of two programs, one that captures unexpectedly and one that doesn't, with the smallest difference possible?

[ggreif]

Maybe this can take cues from #2022?

[nomeata]

Not really, it’s quite indepedent. Do we have a self-contained reproducer yet?

[ggreif]

Nope, but I'll come up with one.

[ggreif]

Here is something silly:

$ cat test/run-drun/capured-module-var.mo

import P "mo:prim";

actor a {

  public func foo (i : Word32) : async Word32 {
    let { popcntWord32 } = P;
    await (async ());
    return popcntWord32 i
  }
}

Compile: make -C test/run-drun _out/capured-module-var_done; observe: wasm2wat test/run-drun/_out/capured-module-var.wasm

In $$lambda.6 a closure is being built:

    ...
    i32.const 6
    call $alloc_words
    local.tee $$k/5_clos
    i32.const 7
    i32.store offset=1
    local.get $$k/5_clos
    i32.const 8
    i32.store offset=5
    local.get $$k/5_clos
    i32.const 3
    i32.store offset=9
    local.get $$k/5_clos
    local.get $$k/4
    i32.store offset=13
    local.get $$k/5_clos
    local.get $$popcntWord32/0
    i32.store offset=17
    local.get $$k/5_clos
    local.get $i
    i32.store offset=21
    local.get $$k/5_clos
    local.set $$k/5
    i32.const 0
    call $@new_async
    ...

When compiling

import P "mo:prim";

actor a {

  public func foo (i : Word32) : async Word32 {
    //let { popcntWord32 } = P;
    await (async ());
    return P.popcntWord32 i
  }
}

I get a direct call to $popcntWord32, and the same closure is only of size 5.

[nomeata]

Great example, thanks!

Ah, it’s a local let { popcntWord32 } = P;, not an import { popcntWord32 } = …. Which makes sense, because we don’t have the latter yet.

[ggreif]

See the first line of my description... :-)

[nomeata]

Yes yes, I mentally translated that to import, because clearly that’s where you’d want to use this :-)

[nomeata]

I tried to minimize it further, but here it works fine:

import P "mo:prim";

func go1 (i : Word32) {
  func capture1() { ignore (P.popcntWord32 1) };
  capture1();
  let { popcntWord32 } = P;
  func capture2() { ignore (popcntWord32 1) };
  capture2();
  func capture3() { ignore (P.popcntWord32 i) };
  capture3();
  func capture4() { ignore (popcntWord32 i) };
  capture4();
};

go1(1);

In all cases, we get a direct call to popcntWord32.

Also in

import P "mo:prim";

func call(clos : () -> ()) = clos();

func go1 (i : Word32) {
  let { popcntWord32 } = P;
  call (func capture3() { ignore (P.popcntWord32 i) });
  call (func capture4() { ignore (popcntWord32 i) });
};

go1(1);

which seems to be even more similar to async/await.

So maybe something in particular to the await translation?

[ggreif]

Yes, the closure arises from the await transform.

[nomeata]

I see this in the output of -dl -v, in await lowering:

        (FuncE
          $lambda
          Local
          Returns
          ($foo Any)
          (params $k/4 $r/2)
          ()
          (DeclareE
            $popcntWord32/0
            (BlockE
              (LetD (ObjP (popcntWord32 (VarP $v/4))) (VarE P))
              (LetD WildP (DefineE $popcntWord32/0 Const (VarE $v/4)))
              (BlockE
                (LetD (VarP $u/3) (PrimE TupPrim))

@crusso, it seems you are using DeclareE and DefineE even for variables that are neither mutable, nor assiged-after-yield. Is that necessary?

And while we are at it; can we avoid all those let $u = (); let _ = $u; … assignments? They make the IR quite hard to read, and it seems quite pointless:

                    (BlockE
                      (LetD (VarP $v/5) (PrimE TupPrim))
                      (BlockE
                        (LetD WildP (VarE $v/5))
…

Ok, I kinda see where they are coming from. This comes from let _ = await …, in the corner case of the await returning no values (so empty tuple), and the pattern match being a wild card, and the fact that the await translation is compositional. Not sure how much can be done there, actually. I guess Claudio is hoping for a separate cleanup IR pass? :-)

[nomeata]

Hmm, looking at the code it seems hard: Looks like all ids bound in a let-pattern are turned into DeclareE/DefineE pairs. And I guess to detect those where it is not necessary is tricky.

Maybe some like: all declarations before, and the right-hand side, are trivial?

Or: there is no use of this before the declarations?

Maybe the proper solution is to finally add a non-recursive let to the IR, and add a pass that turns as many of out let-rec’s into non-recursive lets. Otherwise, all of our passes will have trouble with seemingly trivial things.