Suboptimal reference counting in some matches.

[TimWhiting]

Take for example the following:

effect ask
  fun ask(): string
  fun ask2(): string

fun hello()
  val name = ask()
  println("Hello " ++ name ++ ", " ++ ask2())  // "Hello <ask>, <ask>"

fun main()
  with handler
    fun ask() ""
    fun ask2() "+"
  hello()

The (final core) code to extract a clause looks fine: It only dups the function we extract from the handler.

pub  fun .select-ask : forall<e,a> (^ hnd : scratch/effect/.hnd-ask<e,a>) -> std/core/hnd/clause0<string,scratch/effect/.hnd-ask,e,a>
  = fn(hnd: scratch/effect/.hnd-ask<106,107>){
    match (hnd) {
      (.skip scratch/effect/.Hnd-ask((fun-ask: std/core/hnd/clause0<string,scratch/effect/.hnd-ask,106,107>), (.pat0: std/core/hnd/clause0<string,
   scratch/effect/.hnd-ask,106,107>)) : scratch/effect/.hnd-ask<e,a> )
         -> std/core/types/.dup(fun-ask);
    };
  };

However when this gets inlined into the code to actually call the ask function we get the following, which dups the whole handler, then checks if that handler is unique (which it is obviously not since it just duped it). Followed by dropping / the unused pattern binders, and decrefing the handler. I'm not sure if the fusing of dup / drop should happen through a match based on certain conditions, or if the match binders should just be inserted into the borrowed environment instead of the owned environment since that seems to maybe be the only difference between the .select-ask code's match and the inlined. Maybe it's a combination of the two, it should be inserted into the borrowed environment if it is no longer used after the match?

pub  fun ask : () -> scratch/effect/ask string
  = fn<scratch/effect/ask>(){
    val ev.399 : std/core/hnd/ev<scratch/effect/.hnd-ask>
          = std/core/hnd/.evv-at((std/core/ssize_t(0)));
    std/core/types/.unbox((match (ev.399) {
      (.skip std/core/hnd/Ev<e,a>((.pat0: std/core/hnd/htag<scratch/effect/.hnd-ask>), (m0: std/core/hnd/marker<e,a>), ((.skip std/core/types/.Box((h: 
   scratch/effect/.hnd-ask<e,a>)) : .Box ) as .box-x435: .Box), (.pat1: std/core/hnd/cfc), (.pat2: std/core/hnd/evv<e>)) : std/core/hnd/ev<
   scratch/effect/.hnd-ask> )
         -> val _ : scratch/effect/.hnd-ask<e,a>
                  = std/core/types/.dup(h);
        (match (h) {
          (.skip scratch/effect/.Hnd-ask((fun-ask: std/core/hnd/clause0<string,scratch/effect/.hnd-ask,e,a>), (.pat00: std/core/hnd/clause0<string,
   scratch/effect/.hnd-ask,e,a>)) : scratch/effect/.hnd-ask<e,a> )
             -> val _ : ()
                      = (match ((std/core/types/.is-unique(h))) {
                        (std/core/types/True() : bool )
                           -> val _ : ()
                                    = val _ : ()
                                            = std/core/types/.drop(.pat00);
                                    std/core/types/();
                          std/core/types/.free(h);
                        _
                           -> val _ : ()
                                = val _ : std/core/hnd/clause0<string,scratch/effect/.hnd-ask,e,a>
                                        = std/core/types/.dup(fun-ask);
                                std/core/types/();
                              val _ : ()
                                = std/core/types/.dec-ref(h);
                          std/core/types/();
                      });
            (match (fun-ask) {
              (.skip std/core/hnd/Clause0((.fun-unbox-x438: ((std/core/hnd/marker<1007,1008>, std/core/hnd/ev<a>) -> 1007 e))) : std/core/hnd/clause0<string,
   scratch/effect/.hnd-ask,e,a> )
                 -> .fun-unbox-x438(m0, ev.399);
            });
        });
    }));
  };

[TimWhiting]

@anfelor Do you have an idea whether there is a simple fix? It seems to make inlining counterproductive, especially when there is a borrowed parameter used in a match. And seems like a crucial missed opportunity for fusing the dup of the datatype with drops of it's fields when no reuse is possible (If reuse was possible then that would be different).

This also happens for field accessors on datatypes as well I believe - or anywhere a function gets inlined where the match scrutinee was borrowed in the function that gets inlined.

[anfelor]

Hi Tim! I don't have a simple fix -- as far as I can tell fixing this would unfortunately involve rewriting most of the reuse analysis. I already implemented the FIP analysis to avoid this problem, and it seems not too hard to adapt, but I would still expect this to be a week or two of work. Have you run into a performance problem due to this?

[TimWhiting]

No, I haven't run into big performance problems. It was just something I found as I was looking at the C code for my PhD research.