Bring back type test from Fable 2

[alfonsogarciacaro]

See #2182 (comment)

[ncave]

@alfonsogarciacaro The CI error looks like a deficiency in the AST JS printer, here is a simplified version:

let test (exn: exn) =
    match exn with
    | :? System.NotSupportedException  -> ()
    | :? System.SystemException -> ()
    | Failure _ -> raise exn
    | _ -> ()

gets compiled as:

export function test(exn) {
    if (false) {
    }
    else const activePatternResult59 = FSharp$002ECore_Operators_FailurePattern(exn); // <-- Note the missing block after else
    if (activePatternResult59 != null) {
        throw exn;
    }
    else {
    }
}

[alfonsogarciacaro]

Thanks! Should be fixed now 👍 Do you want to merge this already or test it as a separate PR for now?

[ncave]

@alfonsogarciacaro
Looks good, performance with both #2188 (new map/set) and #2190 (this one) together is back to normal, you can go ahead and merge both.

[alfonsogarciacaro]

Thanks a lot for the confirmation, I will merge the PRs 👍

So the performance didn't go up even a little bit? 😄 Btw, did you notice any performance improvement in the JS code generated by Nagareyama compared to Fable 2?

Also, do you think it would be possible/make sense to rebase the service_slim branch of your fcs fork to latest? The Fable compilation part (in the dotnet tool) has improved a lot compared to Fable 2 but the FCS ParseAndCheck time remains the same, so it'd be nice to see if the latest work done on the F# compiler brings better numbers.

[ncave]

@alfonsogarciacaro Performance of generated code is more or less the same (on our benchmark), but that is expected. I don't think there was any major change in JS representation besides using classes by default now, so keeping the same speed on complex code base is a good news. Updating the Map and Set to faster versions may show improvements on benchmarks that use them more heavily, or on micro-benchmarks.

The Fable 3 compilation performance, on the other side, is clearly better than Fable 2, as the non-FCS portion of it is faster by 20%, and there is no after-processing step (Babel etc.).
I'll rebase FCS to latest, I know you were waiting for the witnesses, but that may take time to sort out.
Also, switching to .NET 5 may speed Fable 3 up somewhat more too.

[alfonsogarciacaro]

Thanks! Yes, I was hoping for the witnesses but I guess they're prioritizing now the F# 5 release, which is understandable. With the witnesses we shouldn't need to inline code to solve SRTP so we could precompile the packages instead of putting all the sources in a single project as we're doing now. This should help with the compilation times as the project to ParseAndCheck would be smaller. It would also make it easier to load dependencies into the REPL replacing all the hacks that we do now.

But this is something that should be mostly transparent for the users so we can delay the feature maybe to Fable 3.1 or so.

I did download the dotnet 5-rc sdk and compiled fable as net5 tool. Not sure if I did something wrong but IIRC compilation was almost twice as slow :/

[ncave]

@alfonsogarciacaro Idk, it's supposed to be twice as fast as 3.1, not the other way ;)