From f97f9de019d979b308ffacb6b63e2f93580c3b85 Mon Sep 17 00:00:00 2001 From: Jimmy Byrd Date: Fri, 28 Jul 2023 11:14:32 -0400 Subject: [PATCH] Reducing memory allocations for file path maths (#1139) --- paket.dependencies | 3 +- paket.lock | 7 + src/Directory.Build.props | 3 + .../CompilerServiceInterface.fs | 6 + src/FsAutoComplete.Core/FileSystem.fs | 20 +- src/FsAutoComplete.Core/Utils.fs | 213 +++++++++++------- src/FsAutoComplete.Core/paket.references | 2 + .../LspServers/AdaptiveFSharpLspServer.fs | 5 +- src/FsAutoComplete/paket.references | 2 + 9 files changed, 166 insertions(+), 95 deletions(-) diff --git a/paket.dependencies b/paket.dependencies index d554c0279..f0eeb9edb 100644 --- a/paket.dependencies +++ b/paket.dependencies @@ -56,7 +56,8 @@ nuget GitHubActionsTestLogger nuget Ionide.LanguageServerProtocol >= 0.4.16 nuget Microsoft.Extensions.Caching.Memory nuget OpenTelemetry.Exporter.OpenTelemetryProtocol >= 1.3.2 - +nuget LinkDotNet.StringBuilder 1.18.0 +nuget CommunityToolkit.HighPerformance diff --git a/paket.lock b/paket.lock index 5989f1e8e..de2df54d3 100644 --- a/paket.lock +++ b/paket.lock @@ -28,6 +28,11 @@ NUGET System.Memory (>= 4.5.4) - restriction: || (&& (== net6.0) (>= net461)) (&& (== net6.0) (< netstandard2.1)) (&& (== net7.0) (>= net461)) (&& (== net7.0) (< netstandard2.1)) (== netstandard2.0) (&& (== netstandard2.1) (>= net461)) System.Threading.Tasks.Extensions (>= 4.5.4) - restriction: || (&& (== net6.0) (>= net461)) (&& (== net6.0) (< netstandard2.1)) (&& (== net7.0) (>= net461)) (&& (== net7.0) (< netstandard2.1)) (== netstandard2.0) (&& (== netstandard2.1) (>= net461)) CommandLineParser (2.4.3) + CommunityToolkit.HighPerformance (7.0.1) + Microsoft.Bcl.HashCode (>= 1.1) - restriction: || (&& (== net6.0) (< netcoreapp2.1) (< netstandard2.1)) (&& (== net7.0) (< netcoreapp2.1) (< netstandard2.1)) (== netstandard2.0) + System.Memory (>= 4.5.4) - restriction: || (&& (== net6.0) (< netcoreapp2.1) (< netstandard2.1)) (&& (== net6.0) (< netstandard2.0)) (&& (== net7.0) (< netcoreapp2.1) (< netstandard2.1)) (&& (== net7.0) (< netstandard2.0)) (== netstandard2.0) (&& (== netstandard2.1) (< netstandard2.0)) + System.Runtime.CompilerServices.Unsafe (>= 5.0) - restriction: || (&& (== net6.0) (< net5.0)) (&& (== net6.0) (< netcoreapp2.1)) (&& (== net6.0) (< netstandard2.0)) (&& (== net6.0) (< netstandard2.1)) (&& (== net7.0) (< net5.0)) (&& (== net7.0) (< netcoreapp2.1)) (&& (== net7.0) (< netstandard2.0)) (&& (== net7.0) (< netstandard2.1)) (== netstandard2.0) (== netstandard2.1) + System.Threading.Tasks.Extensions (>= 4.5.4) - restriction: || (&& (== net6.0) (< netcoreapp2.1) (< netstandard2.1)) (&& (== net6.0) (< netstandard2.0)) (&& (== net7.0) (< netcoreapp2.1) (< netstandard2.1)) (&& (== net7.0) (< netstandard2.0)) (== netstandard2.0) (&& (== netstandard2.1) (< netstandard2.0)) Destructurama.FSharp (1.2) FSharp.Core (>= 4.3.4) Serilog (>= 2.0 < 3.0) @@ -148,6 +153,7 @@ NUGET Ionide.ProjInfo.Sln (>= 0.61.3) - restriction: || (== net6.0) (== net7.0) (&& (== netstandard2.0) (>= net6.0)) (&& (== netstandard2.1) (>= net6.0)) Newtonsoft.Json (>= 13.0.1) - restriction: || (== net6.0) (== net7.0) (&& (== netstandard2.0) (>= net6.0)) (&& (== netstandard2.1) (>= net6.0)) Ionide.ProjInfo.Sln (0.61.3) + LinkDotNet.StringBuilder (1.18) McMaster.NETCore.Plugins (1.4) - restriction: || (== net6.0) (== net7.0) (&& (== netstandard2.0) (>= net5.0)) (&& (== netstandard2.1) (>= net5.0)) Microsoft.DotNet.PlatformAbstractions (>= 3.1.6) - restriction: || (== net6.0) (== net7.0) (&& (== netstandard2.0) (>= netcoreapp2.1)) (&& (== netstandard2.1) (>= netcoreapp2.1)) Microsoft.Extensions.DependencyModel (>= 5.0) - restriction: || (== net6.0) (== net7.0) (&& (== netstandard2.0) (>= netcoreapp2.1)) (&& (== netstandard2.1) (>= netcoreapp2.1)) @@ -162,6 +168,7 @@ NUGET MessagePack.Annotations (2.4.35) Microsoft.Bcl.AsyncInterfaces (6.0) System.Threading.Tasks.Extensions (>= 4.5.4) - restriction: || (&& (== net6.0) (>= net461)) (&& (== net6.0) (< netstandard2.1)) (&& (== net7.0) (>= net461)) (&& (== net7.0) (< netstandard2.1)) (== netstandard2.0) (&& (== netstandard2.1) (>= net461)) + Microsoft.Bcl.HashCode (1.1) - restriction: || (&& (== net6.0) (< netcoreapp2.1) (< netstandard2.1)) (&& (== net7.0) (< netcoreapp2.1) (< netstandard2.1)) (== netstandard2.0) Microsoft.Build (17.2) - copy_local: false Microsoft.Build.Framework (>= 17.2) - restriction: || (== net6.0) (== net7.0) (&& (== netstandard2.0) (>= net472)) (&& (== netstandard2.0) (>= net6.0)) (&& (== netstandard2.1) (>= net472)) (&& (== netstandard2.1) (>= net6.0)) Microsoft.NET.StringTools (>= 1.0) - restriction: || (== net6.0) (== net7.0) (&& (== netstandard2.0) (>= net472)) (&& (== netstandard2.0) (>= net6.0)) (&& (== netstandard2.1) (>= net472)) (&& (== netstandard2.1) (>= net6.0)) diff --git a/src/Directory.Build.props b/src/Directory.Build.props index 0c92968a9..540bb0d78 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -7,5 +7,8 @@ $(NoWarn);FS2003 $(NoWarn);NU5104 + + $(NoWarn);FS3391 + diff --git a/src/FsAutoComplete.Core/CompilerServiceInterface.fs b/src/FsAutoComplete.Core/CompilerServiceInterface.fs index 1ea00f523..a167bbc14 100644 --- a/src/FsAutoComplete.Core/CompilerServiceInterface.fs +++ b/src/FsAutoComplete.Core/CompilerServiceInterface.fs @@ -325,6 +325,12 @@ type FSharpCompilerServiceChecker(hasAnalyzers, typecheckCacheSize) = else return r with ex -> + checkerLogger.error ( + Log.setMessage "{opName} completed with exception: {ex}" + >> Log.addContextDestructured "opName" opName + >> Log.addExn ex + ) + return! ResultOrString.Error(ex.ToString()) } diff --git a/src/FsAutoComplete.Core/FileSystem.fs b/src/FsAutoComplete.Core/FileSystem.fs index f982ab127..607447c68 100644 --- a/src/FsAutoComplete.Core/FileSystem.fs +++ b/src/FsAutoComplete.Core/FileSystem.fs @@ -822,22 +822,22 @@ type FileSystem(actualFs: IFileSystem, tryFindFile: string -> Volatil member _.IsPathRootedShim(p: string) = let r = isWindowsStyleRootedPath p || isUnixStyleRootedPath p - fsLogger.debug ( - Log.setMessage "Is {path} rooted? {result}" - >> Log.addContext "path" p - >> Log.addContext "result" r - ) + // fsLogger.debug ( + // Log.setMessage "Is {path} rooted? {result}" + // >> Log.addContext "path" p + // >> Log.addContext "result" r + // ) r member _.GetFullPathShim(f: string) = let expanded = Path.FilePathToUri f |> Path.FileUriToLocalPath - fsLogger.debug ( - Log.setMessage "{path} expanded to {expanded}" - >> Log.addContext "path" f - >> Log.addContext "expanded" expanded - ) + // fsLogger.debug ( + // Log.setMessage "{path} expanded to {expanded}" + // >> Log.addContext "path" f + // >> Log.addContext "expanded" expanded + // ) expanded diff --git a/src/FsAutoComplete.Core/Utils.fs b/src/FsAutoComplete.Core/Utils.fs index a8d433032..dfd2b1f9b 100644 --- a/src/FsAutoComplete.Core/Utils.fs +++ b/src/FsAutoComplete.Core/Utils.fs @@ -10,6 +10,10 @@ open FSharp.Compiler.CodeAnalysis open FSharp.UMX open FSharp.Compiler.Symbols open System.Runtime.CompilerServices +open LinkDotNet.StringBuilder +open CommunityToolkit.HighPerformance.Buffers +open FsAutoComplete.Logging +open System.Globalization /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources. @@ -106,39 +110,52 @@ type Document = GetLineText0: int -> string GetLineText1: int -> string } + /// /// Checks if the file ends with `.fsx` `.fsscript` or `.sketchfs` /// -let isAScript (fileName: string) = - let ext = Path.GetExtension(fileName) - - [ ".fsx"; ".fsscript"; ".sketchfs" ] |> List.exists ((=) ext) +let inline isAScript (fileName: ReadOnlySpan) = + fileName.EndsWith ".fsx" + || fileName.EndsWith ".fsscript" + || fileName.EndsWith ".sketchfs" /// /// Checks if the file ends with `.fsi` /// -let isSignatureFile (fileName: string) = fileName.EndsWith ".fsi" +let inline isSignatureFile (fileName: ReadOnlySpan) = fileName.EndsWith ".fsi" /// /// Checks if the file ends with `.fs` /// -let isFsharpFile (fileName: string) = fileName.EndsWith ".fs" +let isFsharpFile (fileName: ReadOnlySpan) = fileName.EndsWith ".fs" + +let inline internal isFileWithFSharpI fileName = + isAScript fileName || isSignatureFile fileName || isFsharpFile fileName + /// /// This is a combination of `isAScript`, `isSignatureFile`, and `isFsharpFile` /// /// /// -let isFileWithFSharp fileName = - [ isAScript; isSignatureFile; isFsharpFile ] - |> List.exists (fun f -> f fileName) - -let normalizePath (file: string) : string = - if isFileWithFSharp file then - let p = Path.GetFullPath file - UMX.tag ((p.Chars 0).ToString().ToLower() + p.Substring(1)) +let inline isFileWithFSharp (fileName: string) = isFileWithFSharpI fileName + +let inline internal normalizePathI (file: ReadOnlySpan) : string = + if isFileWithFSharpI file then + + let p = (Path.GetFullPath(StringPool.Shared.GetOrAdd file)).AsSpan() + let buffer = SpanOwner.Allocate(p.Length) + + try + let written = p.Slice(0, 1).ToLower(buffer.Span, CultureInfo.InvariantCulture) + p.Slice(written).CopyTo(buffer.Span.Slice(written)) + UMX.tag (StringPool.Shared.GetOrAdd buffer.Span) + finally + buffer.Dispose() else - UMX.tag file + UMX.tag (StringPool.Shared.GetOrAdd file) + +let inline normalizePath (file: string) : string = normalizePathI file let inline combinePaths path1 (path2: string) = Path.Combine(path1, path2.TrimStart [| '\\'; '/' |]) @@ -510,84 +527,116 @@ type Path with /// Algorithm from https://stackoverflow.com/a/35734486/433393 for converting file paths to uris, /// modified slightly to not rely on the System.Path members because they vary per-platform static member FilePathToUri(filePath: string) : string = - let filePath, finished = - if filePath.Contains "Untitled-" then - let rg = System.Text.RegularExpressions.Regex.Match(filePath, @"(Untitled-\d+).fsx") - if rg.Success then - rg.Groups.[1].Value, true - else - filePath, false - else - filePath, false + let mutable filePathSpan = filePath.AsSpan() + let mutable finished = false + + if filePathSpan.Contains("Untitled-", StringComparison.InvariantCultureIgnoreCase) then + + let rg = System.Text.RegularExpressions.Regex.Match(filePath, @"(Untitled-\d+).fsx") + + if rg.Success then + filePathSpan <- rg.Groups.[1].Value.AsSpan() + finished <- true if not finished then - let uri = System.Text.StringBuilder(filePath.Length) - - for c in filePath do - if - (c >= 'a' && c <= 'z') - || (c >= 'A' && c <= 'Z') - || (c >= '0' && c <= '9') - || c = '+' - || c = '/' - || c = '.' - || c = '-' - || c = '_' - || c = '~' - || c > '\xFF' - then - uri.Append(c) |> ignore - // handle windows path separator chars. - // we _would_ use Path.DirectorySeparator/AltDirectorySeparator, but those vary per-platform and we want this - // logic to work cross-platform (for tests) - else if c = '\\' then - uri.Append('/') |> ignore - else - uri.Append('%') |> ignore - uri.Append((int c).ToString("X2")) |> ignore + let uri = ValueStringBuilder() + + try + let mutable length = 0 + + for c in filePathSpan do + if + (c >= 'a' && c <= 'z') + || (c >= 'A' && c <= 'Z') + || (c >= '0' && c <= '9') + || c = '+' + || c = '/' + || c = '.' + || c = '-' + || c = '_' + || c = '~' + || c > '\xFF' + then + uri.Append(c) + length <- length + 1 + // handle windows path separator chars. + // we _would_ use Path.DirectorySeparator/AltDirectorySeparator, but those vary per-platform and we want this + // logic to work cross-platform (for tests) + else if c = '\\' then + uri.Append('/') + length <- length + 1 + else + uri.Append('%') + let buffer = SpanOwner.Allocate 2 + let mutable out = 0 + + try + (int c).TryFormat(buffer.Span, &out, "X2") |> ignore + uri.Append(buffer.Span) + finally + buffer.Dispose() + + length <- length + 2 + + let file = + if uri.Length >= 2 && uri.[0] = '/' && uri.[1] = '/' then // UNC path + "file:".AsSpan() + else + uri.TrimStart('/') + "file:///".AsSpan() + + let buffer = SpanOwner.Allocate(file.Length + uri.Length) + let finalResult = ValueStringBuilder(buffer.Span) + + try + finalResult.Append(file) + finalResult.Append(uri.AsSpan()) + + StringPool.Shared.GetOrAdd(finalResult.AsSpan()) + finally + finalResult.Dispose() + buffer.Dispose() + finally + uri.Dispose() - if uri.Length >= 2 && uri.[0] = '/' && uri.[1] = '/' then // UNC path - "file:" + uri.ToString() - else - "file:///" + (uri.ToString()).TrimStart('/') - // handle windows path separator chars. - // we _would_ use Path.DirectorySeparator/AltDirectorySeparator, but those vary per-platform and we want this - // logic to work cross-platform (for tests) else - "untitled:" + filePath + String.Concat("untitled:", filePathSpan) + + + /// a test that checks if the start of the line is a windows-style drive string, for example + /// /d:, /c:, /z:, etc. + static member inline internal IsWindowsStyleDriveLetterMatch(s: ReadOnlySpan) = + let slice = s.Slice(0, 3) + slice.Length = 3 && slice[0] = '/' && Char.IsLetter slice[1] && slice[2] = ':' + /// handles unifying the local-path logic for windows and non-windows paths, /// without doing a check based on what the current system's OS is. static member FileUriToLocalPath(uriString: string) = - /// a test that checks if the start of the line is a windows-style drive string, for example - /// /d:, /c:, /z:, etc. - let isWindowsStyleDriveLetterMatch (s: string) = - match s.[0..2].ToCharArray() with - | [||] - | [| _ |] - | [| _; _ |] -> false - // 26 windows drive letters allowed, only - | [| '/'; driveLetter; ':' |] when Char.IsLetter driveLetter -> true - | _ -> false - - let initialLocalPath = Uri(uriString).LocalPath - - let fn = - if isWindowsStyleDriveLetterMatch initialLocalPath then - let trimmed = initialLocalPath.TrimStart('/') - - let initialDriveLetterCaps = - string (System.Char.ToLower trimmed.[0]) + trimmed.[1..] - - initialDriveLetterCaps + + let uriStringSpan = uriString.AsSpan() + let mutable initialLocalPath = Uri(uriString).LocalPath.AsSpan() + let mutable builder = ValueStringBuilder() + + try + let isWindowPath = Path.IsWindowsStyleDriveLetterMatch initialLocalPath + + if isWindowPath then + initialLocalPath <- initialLocalPath.TrimStart('/') + builder.Append(System.Char.ToLower initialLocalPath.[0]) + builder.Append(initialLocalPath.Slice(1)) else - initialLocalPath + builder.Append initialLocalPath + + if uriStringSpan.StartsWith("untitled:") then + builder.Append(".fsx") + + StringPool.Shared.GetOrAdd(builder.AsSpan()) + finally + builder.Dispose() + - if uriString.StartsWith "untitled:" then - (fn + ".fsx") - else - fn let inline debug msg = Printf.kprintf Debug.WriteLine msg let inline fail msg = Printf.kprintf Debug.Fail msg diff --git a/src/FsAutoComplete.Core/paket.references b/src/FsAutoComplete.Core/paket.references index 64e2b0548..12fbc304d 100644 --- a/src/FsAutoComplete.Core/paket.references +++ b/src/FsAutoComplete.Core/paket.references @@ -17,3 +17,5 @@ Ionide.LanguageServerProtocol Ionide.KeepAChangelog.Tasks Microsoft.Extensions.Caching.Memory Microsoft.CodeAnalysis +LinkDotNet.StringBuilder +CommunityToolkit.HighPerformance diff --git a/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs b/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs index dff95ca0a..2a693dad9 100644 --- a/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs +++ b/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs @@ -18,6 +18,7 @@ open System.Reactive open System.Reactive.Linq open System.Runtime.CompilerServices open System.Runtime.InteropServices +open System.Buffers open FsAutoComplete.Adaptive open FSharp.Control.Reactive @@ -1264,9 +1265,9 @@ type AdaptiveFSharpLspServer match result with | Error e -> - logger.info ( + logger.error ( Log.setMessage "Typecheck failed for {file} with {error}" - >> Log.addContextDestructured "file" file + >> Log.addContextDestructured "file" file.FileName >> Log.addContextDestructured "error" e ) diff --git a/src/FsAutoComplete/paket.references b/src/FsAutoComplete/paket.references index e8e49943b..cec270234 100644 --- a/src/FsAutoComplete/paket.references +++ b/src/FsAutoComplete/paket.references @@ -26,3 +26,5 @@ FSharp.Data.Adaptive Microsoft.Extensions.Caching.Memory OpenTelemetry.Exporter.OpenTelemetryProtocol Microsoft.CodeAnalysis +LinkDotNet.StringBuilder +CommunityToolkit.HighPerformance