Skip to content

Commit

Permalink
Reducing memory allocations for file path maths
Browse files Browse the repository at this point in the history
  • Loading branch information
TheAngryByrd committed Jul 14, 2023
1 parent 5d5ffe8 commit f4c2410
Show file tree
Hide file tree
Showing 9 changed files with 173 additions and 95 deletions.
3 changes: 2 additions & 1 deletion paket.dependencies
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ nuget GitHubActionsTestLogger
nuget Ionide.LanguageServerProtocol >= 0.4.12
nuget Microsoft.Extensions.Caching.Memory
nuget OpenTelemetry.Exporter.OpenTelemetryProtocol >= 1.3.2

nuget LinkDotNet.StringBuilder 1.18.0
nuget CommunityToolkit.HighPerformance



Expand Down
7 changes: 7 additions & 0 deletions paket.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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))
Expand All @@ -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))
Expand Down
3 changes: 3 additions & 0 deletions src/Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,8 @@
<NoWarn>$(NoWarn);FS2003</NoWarn>
<!-- We've got a prerelease dep on System.CommandLine, and NuGet doesn't like that. NuGet can be quiet in this instance. -->
<NoWarn>$(NoWarn);NU5104</NoWarn>
<!-- Implicit convertions for string should be fine https://github.com/fsharp/fslang-design/blob/main/FSharp-6.0/FS-1093-additional-conversions.md#motivation-for-op_implicit-type-directed-conversion -->
<NoWarn>$(NoWarn);FS3391</NoWarn>

</PropertyGroup>
</Project>
6 changes: 6 additions & 0 deletions src/FsAutoComplete.Core/CompilerServiceInterface.fs
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,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())
}

Expand Down
20 changes: 10 additions & 10 deletions src/FsAutoComplete.Core/FileSystem.fs
Original file line number Diff line number Diff line change
Expand Up @@ -824,22 +824,22 @@ type FileSystem(actualFs: IFileSystem, tryFindFile: string<LocalPath> -> 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

Expand Down
220 changes: 138 additions & 82 deletions src/FsAutoComplete.Core/Utils.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -106,39 +110,52 @@ type Document =
GetLineText0: int -> string
GetLineText1: int -> string }


/// <summary>
/// Checks if the file ends with `.fsx` `.fsscript` or `.sketchfs`
/// </summary>
let isAScript (fileName: string) =
let ext = Path.GetExtension(fileName)

[ ".fsx"; ".fsscript"; ".sketchfs" ] |> List.exists ((=) ext)
let inline isAScript (fileName: ReadOnlySpan<char>) =
fileName.EndsWith ".fsx"
|| fileName.EndsWith ".fsscript"
|| fileName.EndsWith ".sketchfs"

/// <summary>
/// Checks if the file ends with `.fsi`
/// </summary>
let isSignatureFile (fileName: string) = fileName.EndsWith ".fsi"
let inline isSignatureFile (fileName: ReadOnlySpan<char>) = fileName.EndsWith ".fsi"

/// <summary>
/// Checks if the file ends with `.fs`
/// </summary>
let isFsharpFile (fileName: string) = fileName.EndsWith ".fs"
let isFsharpFile (fileName: ReadOnlySpan<char>) = fileName.EndsWith ".fs"

let inline internal isFileWithFSharpI fileName =
isAScript fileName || isSignatureFile fileName || isFsharpFile fileName


/// <summary>
/// This is a combination of `isAScript`, `isSignatureFile`, and `isFsharpFile`
/// </summary>
/// <param name="fileName"></param>
/// <returns></returns>
let isFileWithFSharp fileName =
[ isAScript; isSignatureFile; isFsharpFile ]
|> List.exists (fun f -> f fileName)

let normalizePath (file: string) : string<LocalPath> =
if isFileWithFSharp file then
let p = Path.GetFullPath file
UMX.tag<LocalPath> ((p.Chars 0).ToString().ToLower() + p.Substring(1))
let inline isFileWithFSharp (fileName: string) = isFileWithFSharpI fileName

let inline internal normalizePathI (file: ReadOnlySpan<char>) : string<LocalPath> =
if isFileWithFSharpI file then

let p = (Path.GetFullPath(StringPool.Shared.GetOrAdd file)).AsSpan()
let buffer = SpanOwner<char>.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<LocalPath> (StringPool.Shared.GetOrAdd buffer.Span)
finally
buffer.Dispose()
else
UMX.tag<LocalPath> file
UMX.tag<LocalPath> (StringPool.Shared.GetOrAdd file)

let inline normalizePath (file: string) : string<LocalPath> = normalizePathI file

let inline combinePaths path1 (path2: string) =
Path.Combine(path1, path2.TrimStart [| '\\'; '/' |])
Expand Down Expand Up @@ -510,84 +527,123 @@ 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
#if NET6_0
let rg = System.Text.RegularExpressions.Regex.Match(filePath, @"(Untitled-\d+).fsx")

if rg.Success then
filePathSpan <- rg.Groups.[1].Value.AsSpan()
finished <- true
#else
let rg =
System.Text.RegularExpressions.Regex.EnumerateMatches(filePathSpan, @"(Untitled-\d+).fsx")

for m in rg do
filePathSpan <- filePathSpan.Slice(m.Index, m.Length)
finished <- true
#endif
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<char>.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<char>.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<char>) =
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
Expand Down
2 changes: 2 additions & 0 deletions src/FsAutoComplete.Core/paket.references
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,5 @@ Ionide.LanguageServerProtocol
Ionide.KeepAChangelog.Tasks
Microsoft.Extensions.Caching.Memory
Microsoft.CodeAnalysis
LinkDotNet.StringBuilder
CommunityToolkit.HighPerformance
5 changes: 3 additions & 2 deletions src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ open Newtonsoft.Json.Linq
open Ionide.ProjInfo.ProjectSystem
open System.Reactive
open System.Reactive.Linq
open System.Buffers
open FsAutoComplete.Adaptive

open FSharp.Control.Reactive
Expand Down Expand Up @@ -1201,9 +1202,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
)

Expand Down
2 changes: 2 additions & 0 deletions src/FsAutoComplete/paket.references
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,5 @@ FSharp.Data.Adaptive
Microsoft.Extensions.Caching.Memory
OpenTelemetry.Exporter.OpenTelemetryProtocol
Microsoft.CodeAnalysis
LinkDotNet.StringBuilder
CommunityToolkit.HighPerformance

0 comments on commit f4c2410

Please sign in to comment.