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.

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

Some minor perf improvements #16941

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Compiler/AbstractIL/ilwrite.fs
Original file line number Diff line number Diff line change
Expand Up @@ -3215,7 +3215,7 @@ let count f arr =
module FileSystemUtilities =
open System.Reflection
open System.Globalization
let progress = try Environment.GetEnvironmentVariable("FSharp_DebugSetFilePermissions") <> null with _ -> false
let progress = try not (isNull (Environment.GetEnvironmentVariable("FSharp_DebugSetFilePermissions"))) with _ -> false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isNotNull ?

let inline isNotNull (x: 'T) = not (isNull x)


/// Arbitrary value
[<Literal>]
Expand Down
5 changes: 3 additions & 2 deletions src/Compiler/Driver/CompilerDiagnostics.fs
Original file line number Diff line number Diff line change
Expand Up @@ -616,10 +616,11 @@ module OldStyleMessages =
let mutable showParserStackOnParseError = false
#endif

[<return: Struct>]
let (|InvalidArgument|_|) (exn: exn) =
match exn with
| :? ArgumentException as e -> Some e.Message
| _ -> None
| :? ArgumentException as e -> ValueSome e.Message
| _ -> ValueNone

let OutputNameSuggestions (os: StringBuilder) suggestNames suggestionsF idText =
if suggestNames then
Expand Down
5 changes: 3 additions & 2 deletions src/Compiler/Driver/CreateILModule.fs
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,12 @@ module AttributeHelpers =
| Some(Attrib(_, _, [ AttribBoolArg p ], _, _, _, _)) -> Some p
| _ -> None

[<return: Struct>]
let (|ILVersion|_|) (versionString: string) =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are covered in @psfinaki PR likely, and we decided to wait with changes for a bit, since we already have some SO reports in compiler and those need careful verification.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reason to postpone it is here: #16316 (comment)
There were stackoverflows when allocating more things on stack in certain cases. We will need to make sure we aren't hitting (or won't be hitting) it with AP changes.

try
Some(parseILVersion versionString)
ValueSome(parseILVersion versionString)
with e ->
None
ValueNone

//----------------------------------------------------------------------------
// ValidateKeySigningAttributes, GetStrongNameSigner
Expand Down
5 changes: 3 additions & 2 deletions src/Compiler/Facilities/DiagnosticsLogger.fs
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,11 @@ exception StopProcessingExn of exn option with
| StopProcessingExn(Some exn) -> "StopProcessingExn, originally (" + exn.ToString() + ")"
| _ -> "StopProcessingExn"

[<return: Struct>]
let (|StopProcessing|_|) exn =
match exn with
| StopProcessingExn _ -> Some()
| _ -> None
| StopProcessingExn _ -> ValueSome()
| _ -> ValueNone

let StopProcessing<'T> = StopProcessingExn None

Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/Facilities/DiagnosticsLogger.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ val NoSuggestions: Suggestions
/// Thrown when we stop processing the F# Interactive entry or #load.
exception StopProcessingExn of exn option

val (|StopProcessing|_|): exn: exn -> unit option
val (|StopProcessing|_|): exn: exn -> unit voption

val StopProcessing<'T> : exn

Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/Service/TransparentCompiler.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1155,7 +1155,7 @@ type internal TransparentCompiler

//Trace.TraceInformation("\n" + debugGraph)

if Activity.Current <> null then
if not (isNull (Activity.Current)) then
Activity.Current.AddTag("graph", debugGraph) |> ignore

return nodeGraph, graph
Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/TypedTree/TypedTree.fs
Original file line number Diff line number Diff line change
Expand Up @@ -5933,7 +5933,7 @@ type Construct() =
match baseType with
| null -> false
| x when x.IsGenericType -> false
| x when x.DeclaringType <> null -> false
| x when not (isNull (x.DeclaringType)) -> false
| x -> x.FullName = "System.Delegate" || x.FullName = "System.MulticastDelegate"), m))
IsEnum = st.PUntaint((fun st -> st.IsEnum), m)
IsStructOrEnum = st.PUntaint((fun st -> st.IsValueType || st.IsEnum), m)
Expand Down
6 changes: 3 additions & 3 deletions src/Compiler/Utilities/LruCache.fs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type internal LruCache<'TKey, 'TVersion, 'TValue when 'TKey: equality and 'TVers
let weakList = LinkedList<'TKey * 'TVersion * string * ValueLink<'TValue>>()

let rec removeCollected (node: LinkedListNode<_>) =
if node <> null then
if not (isNull node) then
let key, version, label, value = node.Value

match value with
Expand All @@ -63,7 +63,7 @@ type internal LruCache<'TKey, 'TVersion, 'TValue when 'TKey: equality and 'TVers

let mutable node = weakList.Last

while weakList.Count > keepWeakly && node <> null do
while weakList.Count > keepWeakly && not (isNull node) do
let previous = node.Previous
let key, version, label, _ = node.Value
weakList.Remove node
Expand All @@ -80,7 +80,7 @@ type internal LruCache<'TKey, 'TVersion, 'TValue when 'TKey: equality and 'TVers

let mutable anythingWeakened = false

while strongList.Count > keepStrongly && node <> null do
while strongList.Count > keepStrongly && not (isNull node) do
let previous = node.Previous

match node.Value with
Expand Down
17 changes: 9 additions & 8 deletions src/Compiler/Utilities/illib.fs
Original file line number Diff line number Diff line change
Expand Up @@ -639,8 +639,7 @@ module List =
let duplicates (xs: 'T list) =
xs
|> List.groupBy id
|> List.filter (fun (_, elems) -> Seq.length elems > 1)
|> List.map fst
|> List.choose (fun (key, elems) -> if Seq.length elems > 1 then Some key else None)

let internal allEqual (xs: 'T list) =
match xs with
Expand Down Expand Up @@ -802,15 +801,17 @@ module String =
/// Splits a string into substrings based on the strings in the array separators
let split options (separator: string[]) (value: string) = value.Split(separator, options)

[<return: Struct>]
let (|StartsWith|_|) pattern value =
if String.IsNullOrWhiteSpace value then None
elif value.StartsWithOrdinal pattern then Some()
else None
if String.IsNullOrWhiteSpace value then ValueNone
elif value.StartsWithOrdinal pattern then ValueSome()
else ValueNone

[<return: Struct>]
let (|Contains|_|) pattern value =
if String.IsNullOrWhiteSpace value then None
elif value.Contains pattern then Some()
else None
if String.IsNullOrWhiteSpace value then ValueNone
elif value.Contains pattern then ValueSome()
else ValueNone

let getLines (str: string) =
use reader = new StringReader(str)
Expand Down
4 changes: 2 additions & 2 deletions src/Compiler/Utilities/illib.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,9 @@ module internal String =
/// Splits a string into substrings based on the strings in the array separators
val split: options: StringSplitOptions -> separator: string[] -> value: string -> string[]

val (|StartsWith|_|): pattern: string -> value: string -> unit option
val (|StartsWith|_|): pattern: string -> value: string -> unit voption

val (|Contains|_|): pattern: string -> value: string -> unit option
val (|Contains|_|): pattern: string -> value: string -> unit voption

val getLines: str: string -> string[]

Expand Down
4 changes: 3 additions & 1 deletion src/Compiler/Utilities/lib.fs
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,9 @@ type Graph<'Data, 'Id when 'Id : comparison and 'Id : equality>
let tab = Map.ofList nodes
let nodes = List.map snd nodes
do for node in nodes do
node.nodeNeighbours <- edges |> List.filter (fun (x, _y) -> x = node.nodeId) |> List.map (fun (_, nodeId) -> tab[nodeId])
node.nodeNeighbours <-
edges
|> List.choose (fun (x, nodeId) -> if x = node.nodeId then Some tab[nodeId] else None)

member g.GetNodeData nodeId = tab[nodeId].nodeData

Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/pars.fsy
Original file line number Diff line number Diff line change
Expand Up @@ -6633,7 +6633,7 @@ nameop:

identExpr:
| ident
{ if $1.idText = "" then
{ if System.String.IsNullOrEmpty $1.idText then
SynExpr.FromParseError(SynExpr.Ident($1), $1.idRange)
else
SynExpr.Ident($1) }
Expand Down
13 changes: 4 additions & 9 deletions src/fsi/console.fs
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,11 @@ type internal History() =
current <- -1

member _.Add line =
match line with
| null
| "" -> ()
| _ -> list.Add(line)
if not (String.IsNullOrWhiteSpace line) then
list.Add line

member _.AddLast line =
match line with
| null
| "" -> ()
| _ ->
if not (String.IsNullOrWhiteSpace line) then
list.Add(line)
current <- list.Count

Expand Down Expand Up @@ -539,7 +534,7 @@ type internal ReadLineConsole() =
let previous = history.Previous()
history.Next() |> ignore

if previous = "" then
if String.IsNullOrEmpty previous then
setPrompt true
else
setPrompt (previous.EndsWith(";;"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ type StructUnion =

for i=1 to countOfCases do
let t = basicTypes[i%basicTypes.Length]
if t = "" then
if System.String.IsNullOrEmpty t then
codeSb.AppendLine($" | Case{i}") |> ignore
else
codeSb.AppendLine($" | Case{i} of field1_{i}:{t} * field2_{i}:{t}") |> ignore
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,31 +9,35 @@ let private getContent isSignature sourceCode =
let ast = parseSourceCode ("Test.fs", sourceCode)
FileContentMapping.mkFileContent { Idx = 0; FileName = fileName; ParsedInput = ast }

[<return: Struct>]
let private (|TopLevelNamespace|_|) value e =
match e with
| FileContentEntry.TopLevelNamespace(path, content) ->
let combined = String.concat "." path
if combined = value then Some content else None
| _ -> None
if combined = value then ValueSome content else ValueNone
| _ -> ValueNone

[<return: Struct>]
let private (|OpenStatement|_|) value e =
match e with
| FileContentEntry.OpenStatement path ->
let combined = String.concat "." path
if combined = value then Some() else None
| _ -> None
if combined = value then ValueSome() else ValueNone
| _ -> ValueNone

[<return: Struct>]
let private (|PrefixedIdentifier|_|) value e =
match e with
| FileContentEntry.PrefixedIdentifier path ->
let combined = String.concat "." path
if combined = value then Some() else None
| _ -> None
if combined = value then ValueSome() else ValueNone
| _ -> ValueNone

[<return: Struct>]
let private (|NestedModule|_|) value e =
match e with
| FileContentEntry.NestedModule(name, nestedContent) -> if name = value then Some(nestedContent) else None
| _ -> None
| FileContentEntry.NestedModule(name, nestedContent) -> if name = value then ValueSome(nestedContent) else ValueNone
| _ -> ValueNone

[<Test>]
let ``Top level module only exposes namespace`` () =
Expand Down
6 changes: 3 additions & 3 deletions tests/FSharp.Compiler.UnitTests/BuildGraphTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ module BuildGraphTests =
let private createNode () =
let o = obj ()
GraphNode(node {
Assert.shouldBeTrue (o <> null)
Assert.shouldBeTrue (not (isNull o))
return 1
}), WeakReference(o)

Expand Down Expand Up @@ -149,7 +149,7 @@ module BuildGraphTests =
| :? OperationCanceledException as ex ->
ex

Assert.shouldBeTrue(ex <> null)
Assert.shouldBeTrue(not (isNull ex))

[<Fact>]
let ``A request can cancel 2``() =
Expand Down Expand Up @@ -179,7 +179,7 @@ module BuildGraphTests =
| :? OperationCanceledException as ex ->
ex

Assert.shouldBeTrue(ex <> null)
Assert.shouldBeTrue(not (isNull ex))
try task.Wait(1000) |> ignore with | :? TimeoutException -> reraise() | _ -> ()

[<Fact>]
Expand Down
2 changes: 1 addition & 1 deletion tests/FSharp.Test.Utilities/CompilerAssert.fs
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ module rec CompilerAssertHelpers =
func ()
true, None
with e ->
let errorMessage = if e.InnerException <> null then e.InnerException.ToString() else e.ToString()
let errorMessage = if not (isNull (e.InnerException)) then e.InnerException.ToString() else e.ToString()
stderr.Append errorMessage |> ignore
false, Some e
finally
Expand Down
6 changes: 3 additions & 3 deletions tests/scripts/scriptlib.fsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ module Scripting =

let executeProcess fileName arguments =
let processWriteMessage (chan:TextWriter) (message:string) =
if message <> null then
if not (isNull message) then
chan.WriteLine(message)
printfn "%s %s" fileName arguments
let info = ProcessStartInfo(Arguments=arguments, UseShellExecute=false,
Expand Down Expand Up @@ -122,15 +122,15 @@ module Scripting =
cmdArgs.RedirectOutput|> Option.iter (fun f ->
processInfo.RedirectStandardOutput <- true
p.OutputDataReceived.Add (fun ea ->
if ea.Data <> null then
if not (isNull (ea.Data)) then
out.Append(ea.Data + Environment.NewLine) |> ignore
f ea.Data)
)

cmdArgs.RedirectError |> Option.iter (fun f ->
processInfo.RedirectStandardError <- true
p.ErrorDataReceived.Add (fun ea ->
if ea.Data <> null then
if not (isNull (ea.Data)) then
err.Append(ea.Data + Environment.NewLine) |> ignore
f ea.Data)
)
Expand Down
2 changes: 1 addition & 1 deletion tests/service/ProjectAnalysisTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ module Tests.Service.ProjectAnalysisTests

#nowarn "57" // Experimental stuff

let runningOnMono = try System.Type.GetType("Mono.Runtime") <> null with e -> false
let runningOnMono = try not (isNull (System.Type.GetType("Mono.Runtime"))) with e -> false

open NUnit.Framework
open FsUnit
Expand Down