From 0dd766dbd66322f8074c6ef3f2f0d57b01f2ee2a Mon Sep 17 00:00:00 2001 From: Tuomas Hietanen Date: Fri, 5 Jul 2024 14:28:52 +0100 Subject: [PATCH 1/2] Some performance optimizations --- .../ExcelProvider.DesignTime.fs | 71 +++++++++++-------- .../ExcelProvider.Runtime.fs | 24 +++++-- .../ExcelProvider.Tests.fs | 2 +- 3 files changed, 59 insertions(+), 38 deletions(-) diff --git a/src/ExcelProvider.DesignTime/ExcelProvider.DesignTime.fs b/src/ExcelProvider.DesignTime/ExcelProvider.DesignTime.fs index 8083037..86f508e 100644 --- a/src/ExcelProvider.DesignTime/ExcelProvider.DesignTime.fs +++ b/src/ExcelProvider.DesignTime/ExcelProvider.DesignTime.fs @@ -13,22 +13,33 @@ module internal Helpers = // Active patterns & operators for parsing strings let (@?) (s: string) i = - if i >= s.Length then None else Some s.[i] + if i >= s.Length then ValueNone else ValueSome s.[i] - let inline satisfies predicate (charOption: option) = + let inline satisfies predicate (charOption: voption) = match charOption with - | Some c when predicate c -> charOption - | _ -> None + | ValueSome c when predicate c -> charOption + | _ -> ValueNone + [] let (|EOF|_|) = function - | Some _ -> None - | _ -> Some() + | ValueSome _ -> ValueNone + | _ -> ValueSome() + [] let (|LetterDigit|_|) = satisfies Char.IsLetterOrDigit + [] let (|Upper|_|) = satisfies Char.IsUpper + [] let (|Lower|_|) = satisfies Char.IsLower + let inline forall predicate (source : ReadOnlySpan<_>) = + let mutable state = true + let mutable e = source.GetEnumerator() + while state && e.MoveNext() do + state <- predicate e.Current + state + /// Turns a string into a nice PascalCase identifier let niceName (set: System.Collections.Generic.HashSet<_>) (s: string) = if s = s.ToUpper() then @@ -36,42 +47,40 @@ module internal Helpers = else // Starting to parse a new segment let rec restart i = - seq { - match s @? i with - | EOF -> () - | LetterDigit _ & Upper _ -> yield! upperStart i (i + 1) - | LetterDigit _ -> yield! consume i false (i + 1) - | _ -> yield! restart (i + 1) - } + match s @? i with + | EOF -> Seq.empty + | LetterDigit _ & Upper _ -> upperStart i (i + 1) + | LetterDigit _ -> consume i false (i + 1) + | _ -> restart (i + 1) // Parsed first upper case letter, continue either all lower or all upper and upperStart from i = - seq { - match s @? i with - | Upper _ -> yield! consume from true (i + 1) - | Lower _ -> yield! consume from false (i + 1) - | _ -> yield! restart (i + 1) - } + match s @? i with + | Upper _ -> consume from true (i + 1) + | Lower _ -> consume from false (i + 1) + | _ -> restart (i + 1) // Consume are letters of the same kind (either all lower or all upper) and consume from takeUpper i = - seq { match s @? i with - | Lower _ when not takeUpper -> yield! consume from takeUpper (i + 1) - | Upper _ when takeUpper -> yield! consume from takeUpper (i + 1) + | Lower _ when not takeUpper -> consume from takeUpper (i + 1) + | Upper _ when takeUpper -> consume from takeUpper (i + 1) | _ -> - yield from, i - yield! restart i - } + let r1 = struct(from, i) + let r2 = restart i + seq { + yield r1 + yield! r2 + } // Split string into segments and turn them to PascalCase let mutable name = seq { for i1, i2 in restart 0 do - let sub = s.Substring(i1, i2 - i1) + let sub = s.AsSpan(i1, i2 - i1) - if Seq.forall Char.IsLetterOrDigit sub then - yield sub.[0].ToString().ToUpper() + sub.ToLower().Substring(1) + if forall Char.IsLetterOrDigit sub then + yield Char.ToUpper(sub.[0]).ToString() + sub.Slice(1).ToString().ToLower() } |> String.concat "" @@ -334,9 +343,9 @@ type public ExcelProvider(cfg: TypeProviderConfig) as this = let key = (filename, sheetname, range, hasheaders, forcestring) - if dict.ContainsKey(key) then - dict.[key] - else + match dict.TryGetValue key with + | true, kv -> kv + | false, _ -> let res = ProvidedTypeDefinitionExcelCall key dict.[key] <- res res diff --git a/src/ExcelProvider.Runtime/ExcelProvider.Runtime.fs b/src/ExcelProvider.Runtime/ExcelProvider.Runtime.fs index 836dca4..298eac2 100644 --- a/src/ExcelProvider.Runtime/ExcelProvider.Runtime.fs +++ b/src/ExcelProvider.Runtime/ExcelProvider.Runtime.fs @@ -1,5 +1,6 @@ namespace FSharp.Interop.Excel +[] type ExcelFormat = | Xlsx | Csv @@ -18,6 +19,7 @@ open FSharp.Interop.Excel [] module internal ExcelAddressing = + [] type Address = { Sheet: string; Row: int; Column: int } @@ -139,7 +141,7 @@ module internal ExcelAddressing = let workSheetName = if worksheets.Contains sheetname then sheetname - else if sheetname = null || sheetname = "" then + elif isNull sheetname || sheetname = "" then worksheets.[0].TableName //accept TypeProvider without specific SheetName... else failwithf "ExcelProvider: Sheet [%s] does not exist." sheetname @@ -157,7 +159,7 @@ module internal ExcelAddressing = |> Seq.toList let rangeViewsByColumn = - ranges |> Seq.map rangeViewOffsetRecord |> Seq.concat |> Seq.toList + ranges |> Seq.collect rangeViewOffsetRecord |> Seq.toList if rangeViewsByColumn |> Seq.distinctBy fst |> Seq.length < rangeViewsByColumn.Length then failwith "ExcelProvider: Ranges cannot overlap" @@ -197,7 +199,10 @@ module internal ExcelAddressing = #endif let fail action (ex: exn) = let exceptionTypeName = ex.GetType().Name - let message = sprintf "ExcelProvider: Could not %s. %s - %s" action exceptionTypeName (ex.Message) + + let message = + sprintf "ExcelProvider: Could not %s. %s - %s" action exceptionTypeName (ex.Message) + failwith message use stream = @@ -220,7 +225,10 @@ module internal ExcelAddressing = ExcelDataReader.ExcelReaderFactory.CreateBinaryReader(stream) if reader.IsClosed then - fail action (Exception "ExcelProvider: The reader was closed on startup without raising a specific exception") + fail + action + (Exception + "ExcelProvider: The reader was closed on startup without raising a specific exception") reader with ex -> @@ -252,7 +260,10 @@ module internal ExcelAddressing = #endif let fail action (ex: exn) = let exceptionTypeName = ex.GetType().Name - let message = sprintf "ExcelProvider: Could not %s. %s - %s" action exceptionTypeName (ex.Message) + + let message = + sprintf "ExcelProvider: Could not %s. %s - %s" action exceptionTypeName (ex.Message) + failwith message let excelReader = @@ -329,7 +340,8 @@ type Row(documentId, sheetname, rowIndex, getCellValue: int -> int -> obj, colum |> Seq.map (fun kvp -> kvp.Key) |> Seq.tryFind (fun header -> String.Equals(header, columnName, StringComparison.OrdinalIgnoreCase)) |> function - | Some header -> sprintf "ExcelProvider: Column \"%s\" was not found. Did you mean \"%s\"?" columnName header + | Some header -> + sprintf "ExcelProvider: Column \"%s\" was not found. Did you mean \"%s\"?" columnName header | None -> sprintf "ExcelProvider: Column \"%s\" was not found." columnName |> failwith diff --git a/tests/ExcelProvider.Tests/ExcelProvider.Tests.fs b/tests/ExcelProvider.Tests/ExcelProvider.Tests.fs index 30865b9..92df979 100644 --- a/tests/ExcelProvider.Tests/ExcelProvider.Tests.fs +++ b/tests/ExcelProvider.Tests/ExcelProvider.Tests.fs @@ -323,7 +323,7 @@ let ``Cannot create a type referring to a non-existant sheet at runtime``() = //| Choice2Of2 v -> printfn "2 %A" v //printfn "%A" diag let dstr = diag[0].ToString() - dstr.Substring(dstr.IndexOf "typecheck") |> should equal "typecheck error The type provider 'FSharp.Interop.Excel.ExcelProvider.ProviderImplementation+ExcelProvider' reported an error: Sheet [C] does not exist." + dstr.Substring(dstr.IndexOf "typecheck").Replace("ExcelProvider: ", "") |> should equal "typecheck error The type provider 'FSharp.Interop.Excel.ExcelProvider.ProviderImplementation+ExcelProvider' reported an error: Sheet [C] does not exist." [] From 4d4af87888d50bdc8678f8c7032f732b4e315fa1 Mon Sep 17 00:00:00 2001 From: Tuomas Hietanen Date: Sun, 7 Jul 2024 22:27:32 +0100 Subject: [PATCH 2/2] yield needs to be this way for tail-recursion --- src/ExcelProvider.DesignTime/ExcelProvider.DesignTime.fs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/ExcelProvider.DesignTime/ExcelProvider.DesignTime.fs b/src/ExcelProvider.DesignTime/ExcelProvider.DesignTime.fs index 86f508e..42dfa2f 100644 --- a/src/ExcelProvider.DesignTime/ExcelProvider.DesignTime.fs +++ b/src/ExcelProvider.DesignTime/ExcelProvider.DesignTime.fs @@ -66,11 +66,9 @@ module internal Helpers = | Lower _ when not takeUpper -> consume from takeUpper (i + 1) | Upper _ when takeUpper -> consume from takeUpper (i + 1) | _ -> - let r1 = struct(from, i) - let r2 = restart i seq { - yield r1 - yield! r2 + yield struct(from, i) + yield! restart i } // Split string into segments and turn them to PascalCase