Skip to content

Minor allocation optimizations / cleanup#6983

Merged
rokonec merged 1 commit into
dotnet:mainfrom
ladipro:minor-allocation-fixes
Nov 1, 2021
Merged

Minor allocation optimizations / cleanup#6983
rokonec merged 1 commit into
dotnet:mainfrom
ladipro:minor-allocation-fixes

Conversation

@ladipro

@ladipro ladipro commented Oct 22, 2021

Copy link
Copy Markdown
Member

Context

Fixing a few places where we allocated objects unnecessarily. This is by no means exhaustive.

Changes Made

  • GetOrAdd should be given a delegate to a method that creates a new object, not the new object itself. Otherwise we allocate and throw away the object in the "Get" case.
  • When passing a callbacks to a method, a lambda expression should be used even if all arguments are passed-through. Otherwise a new delegate object is allocated on each invocation. Lambdas are cached in static fields by the compiler.

Testing

Existing unit tests.

return existenceCache.Value.GetOrAdd(directory, Directory.Exists);
}

public virtual bool FileExists(string file)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This method was unused.

p => FileUtilities.DirectoryExistsNoThrow(p),
(p, searchPattern) => Directory.GetDirectories(p, searchPattern),
p => AssemblyNameExtension.GetAssemblyNameEx(p),
(string path, ConcurrentDictionary<string, AssemblyMetadata> assemblyMetadataCache, out AssemblyNameExtension[] dependencies, out string[] scatterFiles, out FrameworkNameVersioning frameworkName)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I wonder if there's a more concise way of expressing this. My understanding is that if there is an out parameter then all parameters have to be listed complete with types.

@rainersigwald rainersigwald added this to the MSBuild 17.1 milestone Oct 25, 2021
@Forgind Forgind added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Oct 25, 2021
@rokonec rokonec merged commit 761f06e into dotnet:main Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants