Skip to content

Commit

Permalink
Some code clean-ups and performance improvements (#2799)
Browse files Browse the repository at this point in the history 
Some trivial code clean-up and performance improvements:

isNull instead of "= null": latkin.org/blog/2015/05/18/null-checking-considerations-in-f-its-harder-than-you-think
Structs/ValueTuples on active patterns that doesn't carry lot of data
Seq.map + Seq.concat = Seq.collect
  • Loading branch information
@Thorium
Thorium authored Aug 31, 2024
1 parent 32ee50e commit 3e5e3f0
Show file tree
Hide file tree
Showing 22 changed files with 115 additions and 108 deletions.
110 changes: 59 additions & 51 deletions src/app/Fake.BuildServer.TeamCity/TeamCityInternal.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,28 +222,34 @@ module private JavaPropertiesFile =
| KeyValue of key: string * value: string

module private Parser =
type CharReader = unit -> char option
type CharReader = unit -> char voption

[<return: Struct>]
let inline (|IsWhitespace|_|) c =
match c with
| Some c -> if c = ' ' || c = '\t' || c = '\u00ff' then Some c else None
| None -> None
| ValueSome c ->
if c = ' ' || c = '\t' || c = '\u00ff' then
ValueSome c
else
ValueNone
| ValueNone -> ValueNone

type IsEof =
| Yes = 1y
| No = 0y

let rec readToFirstChar (c: char option) (reader: CharReader) =
let rec readToFirstChar (c: char voption) (reader: CharReader) =
match c with
| IsWhitespace _ -> readToFirstChar (reader ()) reader
| Some '\r'
| Some '\n' -> None, IsEof.No
| Some _ -> c, IsEof.No
| None -> None, IsEof.Yes
| ValueSome '\r'
| ValueSome '\n' -> ValueNone, IsEof.No
| ValueSome _ -> c, IsEof.No
| ValueNone -> ValueNone, IsEof.Yes

[<return: Struct>]
let inline (|EscapeSequence|_|) c =
match c with
| Some c ->
| ValueSome c ->
if
c = 'r'
|| c = 'n'
Expand All @@ -254,10 +260,10 @@ module private JavaPropertiesFile =
|| c = '''
|| c = '\\'
then
Some c
ValueSome c
else
None
| None -> None
ValueNone
| ValueNone -> ValueNone

let inline isHex c =
(c >= '0' && c <= '9') || (c >= 'A' && c <= 'F') || (c >= 'a' && c <= 'f')
Expand All @@ -270,7 +276,9 @@ module private JavaPropertiesFile =
| 't' -> '\t'
| 'u' ->
match reader (), reader (), reader (), reader () with
| Some c1, Some c2, Some c3, Some c4 when isHex c1 && isHex c2 && isHex c3 && isHex c4 ->
| ValueSome c1, ValueSome c2, ValueSome c3, ValueSome c4 when
isHex c1 && isHex c2 && isHex c3 && isHex c4
->
let hex = String([| c1; c2; c3; c4 |])

let value =
Expand All @@ -280,89 +288,89 @@ module private JavaPropertiesFile =
| _ -> failwith "Invalid unicode escape"
| _ -> c

let inline readKey (c: char option) (reader: CharReader) (buffer: StringBuilder) =
let inline readKey (c: char voption) (reader: CharReader) (buffer: StringBuilder) =
let rec recurseEnd (result: string) =
match reader () with
| Some ':'
| Some '='
| ValueSome ':'
| ValueSome '='
| IsWhitespace _ -> recurseEnd result
| Some '\r'
| Some '\n' -> result, false, None, IsEof.No
| None -> result, false, None, IsEof.Yes
| Some c -> result, true, Some c, IsEof.No
| ValueSome '\r'
| ValueSome '\n' -> result, false, ValueNone, IsEof.No
| ValueNone -> result, false, ValueNone, IsEof.Yes
| ValueSome c -> result, true, ValueSome c, IsEof.No

let rec recurse (c: char option) (buffer: StringBuilder) (escaping: bool) =
let rec recurse (c: char voption) (buffer: StringBuilder) (escaping: bool) =
match c with
| EscapeSequence c when escaping ->
let realChar = readEscapeSequence c reader
recurse (reader ()) (buffer.Append(realChar)) false
| Some ' ' -> recurseEnd (buffer.ToString())
| Some ':'
| Some '=' when not escaping -> recurseEnd (buffer.ToString())
| Some '\r'
| Some '\n' -> buffer.ToString(), false, None, IsEof.No
| None -> buffer.ToString(), false, None, IsEof.Yes
| Some '\\' -> recurse (reader ()) buffer true
| Some c -> recurse (reader ()) (buffer.Append(c)) false
| ValueSome ' ' -> recurseEnd (buffer.ToString())
| ValueSome ':'
| ValueSome '=' when not escaping -> recurseEnd (buffer.ToString())
| ValueSome '\r'
| ValueSome '\n' -> buffer.ToString(), false, ValueNone, IsEof.No
| ValueNone -> buffer.ToString(), false, ValueNone, IsEof.Yes
| ValueSome '\\' -> recurse (reader ()) buffer true
| ValueSome c -> recurse (reader ()) (buffer.Append(c)) false

recurse c buffer false

let rec readComment (reader: CharReader) (buffer: StringBuilder) =
match reader () with
| Some '\r'
| Some '\n' -> Some(Comment(buffer.ToString())), IsEof.No
| None -> Some(Comment(buffer.ToString())), IsEof.Yes
| Some c -> readComment reader (buffer.Append(c))
| ValueSome '\r'
| ValueSome '\n' -> Some(Comment(buffer.ToString())), IsEof.No
| ValueNone -> Some(Comment(buffer.ToString())), IsEof.Yes
| ValueSome c -> readComment reader (buffer.Append(c))

let inline readValue (c: char option) (reader: CharReader) (buffer: StringBuilder) =
let rec recurse (c: char option) (buffer: StringBuilder) (escaping: bool) (cr: bool) (lineStart: bool) =
let inline readValue (c: char voption) (reader: CharReader) (buffer: StringBuilder) =
let rec recurse (c: char voption) (buffer: StringBuilder) (escaping: bool) (cr: bool) (lineStart: bool) =
match c with
| EscapeSequence c when escaping ->
let realChar = readEscapeSequence c reader
recurse (reader ()) (buffer.Append(realChar)) false false false
| Some '\r'
| Some '\n' ->
if escaping || (cr && c = Some '\n') then
recurse (reader ()) buffer false (c = Some '\r') true
| ValueSome '\r'
| ValueSome '\n' ->
if escaping || (cr && c = ValueSome '\n') then
recurse (reader ()) buffer false (c = ValueSome '\r') true
else
buffer.ToString(), IsEof.No
| None -> buffer.ToString(), IsEof.Yes
| Some _ when lineStart ->
| ValueNone -> buffer.ToString(), IsEof.Yes
| ValueSome _ when lineStart ->
let firstChar, _ = readToFirstChar c reader
recurse firstChar buffer false false false
| Some '\\' -> recurse (reader ()) buffer true false false
| Some c -> recurse (reader ()) (buffer.Append(c)) false false false
| ValueSome '\\' -> recurse (reader ()) buffer true false false
| ValueSome c -> recurse (reader ()) (buffer.Append(c)) false false false

recurse c buffer false false true

let rec readLine (reader: CharReader) (buffer: StringBuilder) =
match readToFirstChar (reader ()) reader with
| Some '#', _
| Some '!', _ -> readComment reader (buffer.Clear())
| Some firstChar, _ ->
let key, hasValue, c, isEof = readKey (Some firstChar) reader (buffer.Clear())
| ValueSome '#', _
| ValueSome '!', _ -> readComment reader (buffer.Clear())
| ValueSome firstChar, _ ->
let key, hasValue, c, isEof = readKey (ValueSome firstChar) reader (buffer.Clear())

let value, isEof =
if hasValue then
// We know that we aren't at the end of the buffer, but readKey can return None if it didn't need the next char
let firstChar =
match c with
| Some c -> Some c
| None -> reader ()
| ValueSome c -> ValueSome c
| ValueNone -> reader ()

readValue firstChar reader (buffer.Clear())
else
"", isEof

Some(KeyValue(key, value)), isEof
| None, isEof -> None, isEof
| ValueNone, isEof -> None, isEof

let inline textReaderToReader (reader: TextReader) =
let buffer = [| '\u0000' |]

fun () ->
let eof = reader.Read(buffer, 0, 1) = 0
if eof then None else Some(buffer[0])
if eof then ValueNone else ValueSome(buffer[0])

let parseWithReader reader =
let buffer = StringBuilder(255)
Expand Down
4 changes: 2 additions & 2 deletions src/app/Fake.Core.CommandLineParsing/docopt.fs/Docopt/Docopt.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ module DocHelper =
let title = line'.Substring(0, idxCol)

let sectionName =
let startIdx = title.IndexOf("[")
let endIdx = title.IndexOf("]")
let startIdx = title.IndexOf '['
let endIdx = title.IndexOf ']'

if startIdx <> -1 && endIdx > startIdx then
title.Substring(startIdx + 1, endIdx - startIdx - 1)
Expand Down
2 changes: 1 addition & 1 deletion src/app/Fake.Core.Environment/Environment.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ module Environment =
/// </summary>
let getNewestTool possibleToolPaths =
possibleToolPaths
|> Seq.sortBy (fun p -> p)
|> Seq.sortBy id
|> Array.ofSeq
|> Array.rev
|> Seq.ofArray
Expand Down
4 changes: 2 additions & 2 deletions src/app/Fake.Core.Process/InternalStreams.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ module internal InternalStreams =
(fun res -> endAction res),
cancelAction =
(fun () ->
while asyncResult.Value = null do
while isNull asyncResult.Value do
Thread.Sleep 20

cancelAction (asyncResult.Value))
Expand Down Expand Up @@ -265,7 +265,7 @@ module internal InternalStreams =
data <- resultData
event.Set() |> ignore

if callback <> null then
if not (isNull callback) then
callback.Invoke(x :> IAsyncResult)

completed <- true)
Expand Down
29 changes: 17 additions & 12 deletions src/app/Fake.Core.SemVer/SemVer.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,6 @@ open System.Text.RegularExpressions
/// Contains active patterns which allow to deal with <a href="http://semver.org/">Semantic Versioning (SemVer)</a>.
/// </summary>
module SemVerActivePattern =
let (|ParseRegex|_|) pattern input =
let m = Regex.Match(input, pattern, RegexOptions.ExplicitCapture)

match m.Success with
| true -> Some(List.tail [ for g in m.Groups -> g.Value ])
| false -> None

[<Literal>]
let Pattern =
Expand All @@ -25,10 +19,19 @@ module SemVerActivePattern =
+ @"(\-(?<pre>[0-9A-Za-z\-\.]+))?"
+ @"(\+(?<build>[0-9A-Za-z\-\.]+))?$"

let RegMatch = Regex(Pattern, RegexOptions.ExplicitCapture)

let (|ParseRegex|_|) input =
let m = RegMatch.Match input

match m.Success with
| true -> Some(List.tail [ for g in m.Groups -> g.Value ])
| false -> None

let (|SemVer|_|) version =

match version with
| ParseRegex Pattern [ major; minor; patch; pre; build ] -> Some [ major; minor; patch; pre; build ]
| ParseRegex [ major; minor; patch; pre; build ] -> Some [ major; minor; patch; pre; build ]
| _ -> None

let (|ValidVersion|_|) =
Expand Down Expand Up @@ -273,18 +276,20 @@ module SemVer =
/// <summary>
/// Matches if str is convertible to Int and not less than zero, and returns the value as UInt.
/// </summary>
[<return: Struct>]
let inline private (|Int|_|) (str: string) =
match Int32.TryParse(str, NumberStyles.Integer, null) with
| true, num when num > -1 -> Some num
| _ -> None
| true, num when num > -1 -> ValueSome num
| _ -> ValueNone

/// <summary>
/// Matches if str is convertible to big int and not less than zero, and returns the bigint value.
/// </summary>
[<return: Struct>]
let inline private (|Big|_|) (str: string) =
match BigInteger.TryParse(str, NumberStyles.Integer, null) with
| true, big when big > -1I -> Some big
| _ -> None
| true, big when big > -1I -> ValueSome big
| _ -> ValueNone

/// <summary>
/// Splits the given version string by possible delimiters but keeps them as parts of resulting list.
Expand Down Expand Up @@ -340,7 +345,7 @@ module SemVer =
if version.Contains("..") then
failwithf "Empty version part found in %s" version

let plusIndex = version.IndexOf("+")
let plusIndex = version.IndexOf '+'

let versionStr =
match plusIndex with
Expand Down
2 changes: 1 addition & 1 deletion src/app/Fake.Core.String/String.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ module String =
/// Removes all trailing .0 from a version string
/// </summary>
let rec NormalizeVersion (version: string) =
if version = null then
if isNull version then
""
else
let elements = version.Split [| '.' |]
Expand Down
6 changes: 3 additions & 3 deletions src/app/Fake.Core.String/StringBuilder.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ module StringBuilder =
/// </summary>
let inline appendWithoutQuotesIfNotNull (value: Object) s =
appendIfTrueWithoutQuotes
(value <> null)
(not (isNull value))
(match value with
| :? String as sv -> (sprintf "%s%s" s sv)
| _ -> (sprintf "%s%A" s value))
Expand All @@ -58,7 +58,7 @@ module StringBuilder =
/// </summary>
let inline appendIfNotNull (value: Object) s =
appendIfTrue
(value <> null)
(not (isNull value))
(match value with
| :? String as sv -> (sprintf "%s%s" s sv)
| _ -> (sprintf "%s%A" s value))
Expand All @@ -67,7 +67,7 @@ module StringBuilder =
/// Appends a quoted text if the value is not null.
/// </summary>
let inline appendQuotedIfNotNull (value: Object) s (builder: StringBuilder) =
if (value = null) then
if isNull value then
builder
else
(match value with
Expand Down
2 changes: 1 addition & 1 deletion src/app/Fake.Core.Target/Target.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -861,7 +861,7 @@ module Target =
let isValidTarget name = targetLeftSet.Contains(name)

let canBeExecuted (t: Target) =
t.Dependencies @ t.SoftDependencies |> Seq.filter isValidTarget |> Seq.isEmpty
t.Dependencies @ t.SoftDependencies |> Seq.exists isValidTarget |> not

let map =
targetLeft
Expand Down
Loading

0 comments on commit 3e5e3f0

Please sign in to comment.