Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Optimized List module #2124

Closed
wants to merge 5 commits into from
Closed

[WIP] Optimized List module #2124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bba7df4
Move List object to List module
ncave Aug 4, 2020
806cd63
List module update
ncave Aug 5, 2020
3c1d850
Optimized IEnumerator.get_Current
ncave Aug 11, 2020
cdf6b30
Optimized List.reverse
ncave Aug 11, 2020
2313047
Fixed Seq.compareWith
ncave Aug 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/Fable.Transforms/FSharp2Fable.Util.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1039,7 +1039,10 @@ module Util =
| [arg] when memb.IsPropertySetterMethod ->
let t = memb.CurriedParameterGroups.[0].[0].Type |> makeType com Map.empty
Fable.Set(callee, Fable.FieldSet(name, t), arg, r)
| _ when memb.IsPropertyGetterMethod && countNonCurriedParams memb = 0 ->
| _ when memb.IsPropertyGetterMethod && countNonCurriedParams memb = 0
// performance optimization, compile get_Current as instance call instead of a getter
&& memb.FullName <> "System.Collections.IEnumerator.get_Current"
&& memb.FullName <> "System.Collections.Generic.IEnumerator.get_Current" ->
let t = memb.ReturnParameter.Type |> makeType com Map.empty
let kind = Fable.FieldGet(name, true, t)
Fable.Get(callee, kind, typ, r)
Expand Down
19 changes: 14 additions & 5 deletions src/Fable.Transforms/FSharp2Fable.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,15 +171,20 @@ let private transformTraitCall com (ctx: Context) r typ (sourceTypes: FSharpType
let private transformObjExpr (com: IFableCompiler) (ctx: Context) (objType: FSharpType)
baseCallExpr (overrides: FSharpObjectExprOverride list) otherOverrides =

let mapOverride (over: FSharpObjectExprOverride) =
let mapOverride (typ: FSharpType) (over: FSharpObjectExprOverride) =
trampoline {
let ctx, args = bindMemberArgs com ctx over.CurriedParameterGroups
let! body = transformExpr com ctx over.Body
let value = Fable.Function(Fable.Delegate args, body, None)
let name, kind =
match over.Signature.Name with
| Naming.StartsWith "get_" name when countNonCurriedParamsForOverride over = 0 ->
name, Fable.ObjectGetter
// performance optimization, compile get_Current as instance call instead of a getter
if over.Signature.Name = "get_Current" &&
(typ.TypeDefinition.FullName = "System.Collections.Generic.IEnumerator`1" ||
typ.TypeDefinition.FullName = "System.Collections.IEnumerator")
then name, Fable.ObjectMethod false
else name, Fable.ObjectGetter
Copy link
Member

@alfonsogarciacaro alfonsogarciacaro Aug 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding ad-hoc exceptions, we can get this "for free" in Fable 3 by removing Enumerator from the list of non-mangled System interfaces here (though in that case we need to change the TypeScript code that uses it) as getters/setters in mangled interfaces compile to methods:

Fable/src/Fable.Transforms/FSharp2Fable.Util.fs

Lines 1030 to 1046 in e84aa4d

let isMangledAbstractEntity (ent: FSharpEntity) =
match ent.TryFullName with
// By default mangle interfaces in System namespace as they are not meant to interact with JS
// except those that are used in fable-library Typescript files
| Some fullName when fullName.StartsWith("System.") ->
match fullName with
| Types.object
| Types.idisposable
| Types.icomparable
| "System.IObservable`1"
| "System.IObserver`1"
| Types.ienumerableGeneric
| Types.ienumerator
// These are used for injections
| Types.comparer
| Types.equalityComparer -> false
| _ -> true

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alfonsogarciacaro I'm not sure we can do that reliably without replicating the name mangling in TS. Also, this PR targets v.current, not v.next. Anyway, this is still WIP, since the perf in this PR is still below the original Types.List.

| Naming.StartsWith "set_" name when countNonCurriedParamsForOverride over = 1 ->
name, Fable.ObjectSetter
| name ->
Expand Down Expand Up @@ -222,8 +227,8 @@ let private transformObjExpr (com: IFableCompiler) (ctx: Context) (objType: FSha

let! members =
(objType, overrides)::otherOverrides
|> trampolineListMap (fun (_typ, overrides) ->
overrides |> trampolineListMap mapOverride)
|> trampolineListMap (fun (typ, overrides) ->
overrides |> trampolineListMap (mapOverride typ))

return Fable.ObjectExpr(members |> List.concat, makeType com ctx.GenericArgs objType, baseCall)
}
Expand Down Expand Up @@ -985,7 +990,11 @@ let private transformAttachedMember (com: FableCompiler) (ctx: Context)
let kind =
match args with
| [_thisArg; unitArg] when memb.IsPropertyGetterMethod && unitArg.Type = Fable.Unit ->
Fable.ObjectGetter
// performance optimization, compile get_Current as instance call instead of a getter
if (memb.CompiledName = "System-Collections-Generic-IEnumerator`1-get_Current" ||
memb.CompiledName = "System-Collections-IEnumerator-get_Current")
then Fable.ObjectMethod false
else Fable.ObjectGetter
| [_thisArg; _valueArg] when memb.IsPropertySetterMethod ->
Fable.ObjectSetter
| _ when memb.CompiledName = "System-Collections-Generic-IEnumerable`1-GetEnumerator" ->
Expand Down
23 changes: 15 additions & 8 deletions src/Fable.Transforms/Fable2Babel.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ module Util =
makeNativeTypeAnnotation com ctx [genArg] "Array"

and makeListTypeAnnotation com ctx genArg =
makeImportTypeAnnotation com ctx [genArg] "Types" "List"
makeImportTypeAnnotation com ctx [genArg] "List" "List"

and makeUnionTypeAnnotation com ctx genArgs =
List.map (typeAnnotation com ctx) genArgs
Expand Down Expand Up @@ -809,12 +809,16 @@ module Util =
// Optimization for bundle size: compile list literals as List.ofArray
| Replacements.ListLiteral(exprs, t) ->
[|List.rev exprs |> makeArray com ctx|]
|> coreLibCall com ctx r "Types" "newList"
|> coreLibCall com ctx r "List" "newList"
// match exprs with
// | [] -> coreLibCall com ctx r "List" "empty" [||]
// | [TransformExpr com ctx expr] -> coreLibCall com ctx r "List" "singleton" [|expr|]
// | exprs -> [|makeArray com ctx exprs|] |> coreLibCall com ctx r "List" "ofArray"
| Fable.NewList (headAndTail, _) ->
match headAndTail with
| None -> coreLibCall com ctx r "Types" "newList" [||]
| None -> coreLibCall com ctx r "List" "empty" [||]
| Some(TransformExpr com ctx head, TransformExpr com ctx tail) ->
coreLibCall com ctx r "Types" "cons" [|head; tail|]
coreLibCall com ctx r "List" "cons" [|head; tail|]
| Fable.NewOption (value, t) ->
match value with
| Some (TransformExpr com ctx e) ->
Expand Down Expand Up @@ -1054,8 +1058,10 @@ module Util =
let expr = com.TransformAsExpr(ctx, fableExpr)
match getKind with
| Fable.ExprGet(TransformExpr com ctx prop) -> getExpr range expr prop
| Fable.ListHead -> get range expr "Head"
| Fable.ListTail -> get range expr "Tail"
// | Fable.ListHead -> get range expr "Head"
// | Fable.ListTail -> get range expr "Tail"
| Fable.ListHead -> coreLibCall com ctx range "List" "head" [|expr|]
| Fable.ListTail -> coreLibCall com ctx range "List" "tail" [|expr|]
| Fable.FieldGet(fieldName,_,_) ->
let expr =
match fableExpr with
Expand Down Expand Up @@ -1151,7 +1157,7 @@ module Util =
| Fable.FunctionType _ -> jsTypeof "function" expr
| Fable.Array _ | Fable.Tuple _ ->
coreLibCall com ctx None "Util" "isArrayLike" [|com.TransformAsExpr(ctx, expr)|]
| Fable.List _ -> jsInstanceof (coreValue com ctx "Types" "List") expr
| Fable.List _ -> jsInstanceof (coreValue com ctx "List" "List") expr
| Replacements.Builtin kind ->
match kind with
| Replacements.BclGuid -> jsTypeof "string" expr
Expand Down Expand Up @@ -1212,7 +1218,8 @@ module Util =
let op = if nonEmpty then BinaryUnequal else BinaryEqual
upcast BinaryExpression(op, com.TransformAsExpr(ctx, expr), NullLiteral(), ?loc=range)
| Fable.ListTest nonEmpty ->
let expr = get range (com.TransformAsExpr(ctx, expr)) "IsEmpty"
// let expr = get range (com.TransformAsExpr(ctx, expr)) "IsEmpty"
let expr = coreLibCall com ctx range "List" "isEmpty" [|com.TransformAsExpr(ctx, expr)|]
if nonEmpty then upcast UnaryExpression(UnaryNot, expr, ?loc=range)
else expr
| Fable.UnionCaseTest(uci, ent) ->
Expand Down
30 changes: 14 additions & 16 deletions src/Fable.Transforms/Replacements.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,8 @@ let identityHash r (arg: Expr) =
Helper.CoreCall("Util", "structuralHash", Number Int32, [arg], ?loc=r)
| DeclaredType(ent,_) when ent.IsFSharpUnion || ent.IsFSharpRecord || ent.IsValueType ->
Helper.CoreCall("Util", "structuralHash", Number Int32, [arg], ?loc=r)
| DeclaredType(ent,_) ->
Helper.InstanceCall(arg, "GetHashCode", Number Int32, [], ?loc=r)
| _ ->
Helper.CoreCall("Util", "identityHash", Number Int32, [arg], ?loc=r)

Expand Down Expand Up @@ -1838,30 +1840,26 @@ let arrayModule (com: ICompiler) (ctx: Context) r (t: Type) (i: CallInfo) (_: Ex
Helper.CoreCall("Array", meth, t, args, i.SignatureArgTypes, ?loc=r) |> Some

let lists (com: ICompiler) (ctx: Context) r (t: Type) (i: CallInfo) (thisArg: Expr option) (args: Expr list) =
let meth = Naming.removeGetSetPrefix i.CompiledName |> Naming.lowerFirst
match i.CompiledName, thisArg, args with
// Use methods for Head and Tail (instead of Get(ListHead) for example) to check for empty lists
| ("get_Head" | "get_Tail" | "get_Length" | "get_IsEmpty"), Some callee, _ ->
let meth = Naming.removeGetSetPrefix i.CompiledName
get r t callee meth |> Some
| "get_Item", Some callee, _ ->
let meth = Naming.removeGetSetPrefix i.CompiledName
Helper.InstanceCall(callee, meth, t, args, i.SignatureArgTypes, ?loc=r) |> Some
| "GetSlice", Some x, _ ->
let meth = Naming.lowerFirst i.CompiledName
let args = match args with [ExprType Unit] -> [x] | args -> args @ [x]
| ("get_Head" | "get_Tail" | "get_IsEmpty" | "get_Length"), Some x, _ ->
Helper.CoreCall("List", meth, t, [x], i.SignatureArgTypes, ?loc=r) |> Some
// get r t x meth |> Some
| ("get_Item" | "GetSlice"), Some x, _ ->
Helper.CoreCall("List", meth, t, args @ [x], i.SignatureArgTypes, ?loc=r) |> Some
| ("get_Empty" | "Cons"), None, _ ->
Helper.CoreCall("List", meth, t, args, i.SignatureArgTypes, ?loc=r) |> Some
| "get_Empty", None, _ -> NewList(None, (genArg com ctx r 0 i.GenericArgs)) |> makeValue r |> Some
| "Cons", None, [h;t] -> NewList(Some(h,t), (genArg com ctx r 0 i.GenericArgs)) |> makeValue r |> Some
| ("GetHashCode" | "Equals" | "CompareTo"), Some callee, _ ->
Helper.InstanceCall(callee, i.CompiledName, t, args, i.SignatureArgTypes, ?loc=r) |> Some
| _ -> None

let listModule (com: ICompiler) (ctx: Context) r (t: Type) (i: CallInfo) (_: Expr option) (args: Expr list) =
match i.CompiledName, args with
| "IsEmpty", [x] -> Test(x, ListTest false, r) |> Some
| "Empty", _ -> NewList(None, (genArg com ctx r 0 i.GenericArgs)) |> makeValue r |> Some
| "Singleton", [x] ->
NewList(Some(x, Value(NewList(None, t), None)), (genArg com ctx r 0 i.GenericArgs)) |> makeValue r |> Some
// | ("Head" | "Tail" | "IsEmpty") as meth, [x] -> get r t x (Naming.lowerFirst meth) |> Some
// | "IsEmpty", [x] -> Test(x, ListTest false, r) |> Some
// | "Empty", _ -> NewList(None, (genArg com ctx r 0 i.GenericArgs)) |> makeValue r |> Some
// | "Singleton", [x] ->
// NewList(Some(x, Value(NewList(None, t), None)), (genArg com ctx r 0 i.GenericArgs)) |> makeValue r |> Some
// Use a cast to give it better chances of optimization (e.g. converting list
// literals to arrays) after the beta reduction pass
| "ToSeq", [x] -> toSeq t x |> Some
Expand Down
11 changes: 5 additions & 6 deletions src/fable-library/Array.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ module Helpers =
open Helpers

let private indexNotFoundMsg = "An index satisfying the predicate was not found in the collection."
let inline indexNotFound() = failwith indexNotFoundMsg

// Pay attention when benchmarking to append and filter functions below
// if implementing via native JS array .concat() and .filter() do not fall behind due to js-native transitions.
Expand Down Expand Up @@ -478,15 +477,15 @@ let partition (f: 'T -> bool) (source: 'T[]) ([<Inject>] cons: IArrayCons<'T>) =
let find (predicate: 'T -> bool) (array: 'T[]): 'T =
match findImpl predicate array with
| Some res -> res
| None -> indexNotFound()
| None -> failwith indexNotFoundMsg

let tryFind (predicate: 'T -> bool) (array: 'T[]): 'T option =
findImpl predicate array

let findIndex (predicate: 'T -> bool) (array: 'T[]): int =
match findIndexImpl predicate array with
| index when index > -1 -> index
| _ -> indexNotFound()
| _ -> failwith indexNotFoundMsg

let tryFindIndex (predicate: 'T -> bool) (array: 'T[]): int option =
match findIndexImpl predicate array with
Expand All @@ -496,7 +495,7 @@ let tryFindIndex (predicate: 'T -> bool) (array: 'T[]): int option =
let pick chooser (array: _[]) =
let rec loop i =
if i >= array.Length then
indexNotFound()
failwith indexNotFoundMsg
else
match chooser array.[i] with
| None -> loop(i+1)
Expand All @@ -513,7 +512,7 @@ let tryPick chooser (array: _[]) =

let findBack predicate (array: _[]) =
let rec loop i =
if i < 0 then indexNotFound()
if i < 0 then failwith indexNotFoundMsg
elif predicate array.[i] then array.[i]
else loop (i - 1)
loop (array.Length - 1)
Expand All @@ -534,7 +533,7 @@ let findLastIndex predicate (array: _[]) =

let findIndexBack predicate (array: _[]) =
let rec loop i =
if i < 0 then indexNotFound()
if i < 0 then failwith indexNotFoundMsg
elif predicate array.[i] then i
else loop (i - 1)
loop (array.Length - 1)
Expand Down
Loading