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 1 commit
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
74 changes: 19 additions & 55 deletions src/fable-library/List.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,31 +11,31 @@ let msgListNoMatch = "List did not contain any matching elements"
type List<'T when 'T: comparison>(Count: int, Values: ResizeArray<'T>) =
let mutable hashCode = None

static member inline Empty = new List<'T>(0, ResizeArray<'T>())
static member inline internal Cons (x: 'T, xs: 'T list) = xs.Add(x)
static member Empty = new List<'T>(0, ResizeArray<'T>())
static member internal Cons (x: 'T, xs: 'T list) = xs.Add(x)

member inline internal _.Add(x: 'T) =
member internal _.Add(x: 'T) =
let values =
if Count = Values.Count
then Values
else Values.GetRange(0, Count)
values.Add(x)
new List<'T>(values.Count, values)

member inline _.IsEmpty = Count <= 0
member inline _.Length = Count
member _.IsEmpty = Count <= 0
member _.Length = Count

member inline _.Head =
member _.Head =
if Count > 0
then Values.[Count - 1]
else failwith msgListWasEmpty

member inline _.Tail =
member _.Tail =
if Count > 0
then new List<'T>(Count - 1, Values)
else failwith msgListWasEmpty

member inline _.Item with get(index) =
member _.Item with get(index) =
Values.[Count - 1 - index]

override xs.ToString() =
Expand All @@ -46,7 +46,6 @@ type List<'T when 'T: comparison>(Count: int, Values: ResizeArray<'T>) =
if xs.Length <> ys.Length then false
elif xs.GetHashCode() <> ys.GetHashCode() then false
else Seq.forall2 (Unchecked.equals) xs ys
// else (xs :> System.IComparable).CompareTo(other) = 0

override xs.GetHashCode() =
match hashCode with
Expand All @@ -63,59 +62,25 @@ type List<'T when 'T: comparison>(Count: int, Values: ResizeArray<'T>) =
interface System.IComparable with
member xs.CompareTo(other: obj) =
Seq.compareWith compare xs (other :?> 'T list)
// List.CompareWith compare xs (other :?> 'T list)

interface System.Collections.Generic.IEnumerable<'T> with
member xs.GetEnumerator(): System.Collections.Generic.IEnumerator<'T> =
// let elems = seq { for i=xs.Length - 1 downto 0 do yield Values.[i] }
// elems.GetEnumerator()

// new ListEnumerator<'T>(xs) :> System.Collections.Generic.IEnumerator<'T>

let mutable i = Count
{
new System.Collections.Generic.IEnumerator<'T> with
member _.Current = Values.[i]
interface System.Collections.IEnumerator with
member _.Current: obj = box (Values.[i])
member _.MoveNext() = i <- i - 1; i >= 0
member _.Reset() = i <- Count
interface System.IDisposable with
member _.Dispose(): unit = ()
}
new ListEnumerator<'T>(xs) :> System.Collections.Generic.IEnumerator<'T>

interface System.Collections.IEnumerable with
member xs.GetEnumerator(): System.Collections.IEnumerator =
((xs :> System.Collections.Generic.IEnumerable<'T>).GetEnumerator() :> System.Collections.IEnumerator)

// static member internal CompareWith (comparer: 'T -> 'T -> int) (xs: 'T list) (ys: 'T list): int =
// if obj.ReferenceEquals(xs, ys)
// then 0
// else
// if xs.IsEmpty then
// if ys.IsEmpty then 0 else -1
// elif ys.IsEmpty then 1
// else
// let mutable i = 0
// let mutable result = 0
// if xs.Length > ys.Length then 1
// elif xs.Length < ys.Length then -1
// else
// while i < xs.Length && result = 0 do
// result <- comparer xs.[i] ys.[i]
// i <- i + 1
// result

// and ListEnumerator<'T when 'T: comparison>(xs: List<'T>) =
// let mutable i = -1
// interface System.Collections.Generic.IEnumerator<'T> with
// member __.Current = xs.[i]
// interface System.Collections.IEnumerator with
// member __.Current = box (xs.[i])
// member __.MoveNext() = i <- i + 1; i < xs.Length
// member __.Reset() = i <- -1
// interface System.IDisposable with
// member __.Dispose() = ()
and ListEnumerator<'T when 'T: comparison>(xs: List<'T>) =
let mutable i = -1
interface System.Collections.Generic.IEnumerator<'T> with
member __.Current = xs.[i]
interface System.Collections.IEnumerator with
member __.Current = box (xs.[i])
member __.MoveNext() = i <- i + 1; i < xs.Length
member __.Reset() = i <- -1
interface System.IDisposable with
member __.Dispose() = ()

and 'T list when 'T: comparison = List<'T>

Expand Down Expand Up @@ -152,7 +117,6 @@ let tryLast (xs: 'T list) =

let compareWith (comparer: 'T -> 'T -> int) (xs: 'T list) (ys: 'T list): int =
Seq.compareWith comparer xs ys
//List.CompareWith comparer xs ys

let fold (folder: 'acc -> 'T -> 'acc) (state: 'acc) (xs: 'T list) =
let mutable acc = state
Expand Down
6 changes: 3 additions & 3 deletions src/fable-library/Seq.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Option, some, value } from "./Option";
import { compare, equals, IComparer, IDisposable } from "./Util";

export interface IEnumerator<T> {
Current: T | undefined;
Current(): T | undefined; // intentionally not a getter (for performance reasons)
MoveNext(): boolean;
Reset(): void;
}
Expand Down Expand Up @@ -32,7 +32,7 @@ export class Enumerator<T> implements IEnumerator<T>, IDisposable {
this.current = cur.value;
return !cur.done;
}
get Current() {
Current() {
return this.current;
}
public Reset() {
Expand All @@ -51,7 +51,7 @@ export function toIterator<T>(en: IEnumerator<T>): Iterator<T> {
return {
next() {
return en.MoveNext()
? { done: false, value: en.Current }
? { done: false, value: en.Current() }
: { done: true, value: undefined };
},
} as Iterator<T>;
Expand Down