Fable: Implement replacements for optimization switch

[alfonsogarciacaro]

Follow-up to https://github.com/fable-compiler/Fable/pull/1197#issuecomment-348034660

Replacements needed so the optimization switch can pass the test suite:

• [x] Microsoft.FSharp.Collections.FSharpList::CompareTo
• [x] Microsoft.FSharp.Collections.FSharpList::Equals
• [x] Microsoft.FSharp.Collections.FSharpList::GetHashCode
• [x] Microsoft.FSharp.Core.LanguagePrimitives.ErrorStrings::InputArrayEmptyString
• [x] Microsoft.FSharp.Core.LanguagePrimitives.ErrorStrings::InputMustBeNonNegativeString
• [x] Microsoft.FSharp.Core.LanguagePrimitives.ErrorStrings::InputSequenceEmptyString
• [x] Microsoft.FSharp.Core.LanguagePrimitives.HashCompare::GenericComparisonIntrinsic
• [x] Microsoft.FSharp.Core.LanguagePrimitives.HashCompare::GenericEqualityIntrinsic
• [x] Microsoft.FSharp.Core.LanguagePrimitives.HashCompare::GenericGreaterOrEqualIntrinsic
• [x] Microsoft.FSharp.Core.LanguagePrimitives.HashCompare::GenericGreaterThanIntrinsic
• [x] Microsoft.FSharp.Core.LanguagePrimitives.HashCompare::GenericLessThanIntrinsic
• [x] Microsoft.FSharp.Core.LanguagePrimitives.HashCompare::PhysicalEqualityIntrinsic
• [x] Microsoft.FSharp.Core.LanguagePrimitives::GenericEqualityComparer
• [x] Microsoft.FSharp.Core.LanguagePrimitives::GenericEqualityERComparer
• [x] Microsoft.FSharp.Core.LanguagePrimitives::ParseInt32
• [x] Microsoft.FSharp.Core.LanguagePrimitives::ParseInt64
• [x] Microsoft.FSharp.Core.LanguagePrimitives::ParseUInt32
• [x] Microsoft.FSharp.Core.LanguagePrimitives::ParseUInt64
• [x] Microsoft.FSharp.Core.Operators.OperatorIntrinsics::PowDouble
• [x] Microsoft.FSharp.Core.Operators.OperatorIntrinsics::RangeChar
• [x] Microsoft.FSharp.Core.Operators.OperatorIntrinsics::RangeDouble
• [x] Microsoft.FSharp.Core.Operators::Failure
• [x] System.Array::Copy
• [x] System.Char::Parse
• [x] System.Decimal::Parse
• [x] System.Decimal::op_Explicit
• [x] System.Decimal::op_GreaterThan
• [x] System.Decimal::op_LessThan
• [ ] System.Globalization.CultureInfo::get_InvariantCulture
• [ ] System.Type::GetTypeFromHandle
• [ ] System.Type::GetTypeFromHandle
• [ ] System.Single.parse only accepts a single argument
• [ ] System.Double.parse only accepts a single argument
• [x] hash of a string (i.e. hash "abc") for some reason results in an unconverted ILAsm(["I_call"])
• [ ] runtime error: "get_IsGenericType" is missing (in Reflection and Type tests only).

[Zaid-Ajaj]

let xs = [1;2;3]
let other = [1;2]
xs.CompareTo(other)

Gives:

Compilation error (line 3, col 4): The field, constructor or member 'CompareTo' is not defined

Dotnet fiddle sample

[alfonsogarciacaro]

@Zaid-Ajaj I'm not sure but it's possible that method is hidden to users and it's only brought to light by the optimizer. The method is already implemented so it probably only needs a small variation of your #1268 PR :)

Hmm, I think now that the Equals and CompareTo methods of List can probably be optimized a bit...

[Zaid-Ajaj]

I started with adding "CompareTo" as well since an implementation is already provided by ListClass.ts but I couldn't add a test for it because I got that compile error

[alfonsogarciacaro]

I guess we'll have to skip the test until we know how List.CompareTo pops up. My suspicion is this how the optimizer expands compare list1 list2.

[ncave]

@alfonsogarciacaro As some of those are easier to do with F# code, is it part of v2.0 to enable the build to compile F# code in fable-core? Or is that already in place?

[alfonsogarciacaro]

@ncave Yes, it's already possible to compile F# code in fable-core in the dev2.0 branch. Please check #1323 and #1324. I hope we can move some of the modules to F# for Fable 2.x like Array, List, Map, Set and BigInt.

The code will still be distributed in JS form, so care is needed in some situations. For example in the array module the constructor function needs to be passed to build typed arrays for numeric types.

Another difficulty is tests are broken at the moment in dev2.0 but I hope to fix at least some of them next week.

[ncave]

@alfonsogarciacaro Ideally we should baseline performance before we convert any of the modules to F#, to make sure we're not regressing in performance after.

[alfonsogarciacaro]

Map, Set and BigInt were already converted manually from F# so it shouldn't change much. For Array and List there are only a few methods implemented, the rest default to Seq and builds the Array/List from the result. Replacing this with collection-specific methods _must be_ more performant.

It'd be great to have some performance tests anyways, we could apply them to Fable 1 and Fable 2 to see the difference. I'm not sure I can write them now as there's quite a lot of work to do for the next version :/

[ncave]

@alfonsogarciacaro There is an overall benchmark app here, if there is no time for more targeted benchmarks.

[alfonsogarciacaro]

@ncave Does something change now that https://github.com/Microsoft/visualfsharp/pull/3784 is merged? Do we still need to implement all the missing replacements above?

[ncave]

No change, still needed, the ones already implemented are checked.

[alfonsogarciacaro]

Let's close this as using the optimized AST is bringing other difficulties. We'll reopen it if we want to try this again.

[ncave]

@alfonsogarciacaro The optimize-fcs feature is actually progressing along nicely, I'm able to successfully compile (not yet run) FCS with it, and most of the Fable tests, as long as I use the Fable-FCS (not stock FCS). Can we keep this around, possibly as a discussion?

[alfonsogarciacaro]

Ok, let's reopen it then :)

[ncave]

@alfonsogarciacaro
Closing this as most tests can now compile and pass when compiled with latest fable-compiler-js and --optimize-fcs (using latest fable-standalone after it's published). The last few stragglers were fixed with the supression of inlining of FSharp.Core (in FCS-Fable only for now). What's still failing is a handful of JsInterop tests (importMember etc.), but most of it can be worked around, and we can come back and reopen as individual issues.