Skip to content

Commit

Permalink
Improve Buffer Handling to Reduce GC Pressure
Browse files Browse the repository at this point in the history
**Background**
This is a fix for the issue discussed in #202. The default marshaller for `byte[]` arrays allocates new managed buffers for every native -> managed transition of `Read/WriteFile`. Since these
buffers are typically bigger than 85K, they are allocated on the large object heap (LOH) and requires full, expensive Gen2 garbage collection to collect. In some tests, I saw LOH allocation rates
of 500+ MB/s and 30+% CPU eaten up by GC - not good.

**The Improvement**
By manually marshalling the unmanaged buffers to managed `byte[]` arrays, we can pool/reuse the managed buffers and get away with almost no LOH allocations. This change implements that by
introducing a simple buffer pool that is utilized from `Read/WriteFile`. This is fully backwards compatible with existing applications - the full benefit is realized by using the latest
Dokan-DotNet version.

Here is a comparison of copying out a 1 GB file through DokanNetMirror before and after the change.

**Test Setup/Execution**
```
C:\DokanGCTest>fsutil file createnew 1GBfile.dat 1073741824
C:\DokanGCTest>DokanNetMirror.exe
C:\DokanGCTest>copy N:\DokanGCTest\1GBfile.dat .\copytarget.dat /Y
```

**Before**
341 full Gen2 collections to copy the 1 GB file:
![image](https://user-images.githubusercontent.com/33402265/46068250-a32b1980-c12d-11e8-9853-0ec1a06e0443.png)

**After**
0 gen2 collections (even 0 gen0 collections!) to copy the 1 GB file. You may observe that the CPU% isn't all that different between these two samples. This is because there are very few live
objects on the managed heap in this test. If Dokan was running in a larger server application with lots of live objects on the heap, the difference would be much larger.
![image](https://user-images.githubusercontent.com/33402265/46068257-a6bea080-c12d-11e8-81b5-f54b09dad318.png)

**Further Optimization**
This also opens the door to one further optimization opportunity: If the application is reading/writing the file contents from/to an unmanaged buffer (such as a memory mapped file), there is no
reason to marshall the data from unmanaged, to managed and back to unmanaged. If Dokan exposed the unmanaged `IntPtr `to the file system implementation, it can use `Buffer.MemoryCopy` to perform
the copy directly between two unmanaged buffers.

Since this requires application changes, we expose this new capability as a new sub interface of `IDokanOperations`: `IDokanOperationsUnsafe`. It introduces new overloads to `Read/WriteFile` that
takes `(IntPtr, bufferLength)` instead of `(byte[])`. The application can optionally implement this interface to take advantage of the raw buffers.

The PR also includes changes to tests to cover the new functionality:
- Changes to test infrastructure to mount two drives during testing; one for `IDokanOperations `and one for `IDokanOperationsUnsafe`. Tests select during `Setup()` which one they would like to
run against.
- A new test class for the new unsafe overloads, that reuses all the tests from `FileInfoTests `by running them in the new mode with unmanaged buffers.
- Upgraded MSTest reference versions and converted `DirectoryInfoTests `to use `DataTestMethod`/`DynamicData `instead of the old `DataSource`/XML file because these tests appeared to be flaky in
Appveyor. The new way is neater too.

This contribution from Microsoft is licensed under the MIT license.
  • Loading branch information
hansmsft authored and hans-olav committed Sep 30, 2018
1 parent 27eca48 commit b243cb2
Show file tree
Hide file tree
Showing 15 changed files with 636 additions and 137 deletions.
51 changes: 51 additions & 0 deletions DokanNet.Tests/BufferPoolTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
using System;
using DokanNet.Logging;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace DokanNet.Tests
{
/// <summary>
/// Tests for <see cref="BufferPool"/>.
/// </summary>
[TestClass]
public sealed class BufferPoolTests
{
/// <summary>
/// Rudimentary test for <see cref="BufferPool"/>.
/// </summary>
[TestMethod, TestCategory(TestCategories.Success)]
public void BufferPoolBasicTest()
{
BufferPool pool = new BufferPool();
ILogger logger = new ConsoleLogger();

// Verify buffer is pooled.
const int MB = 1024 * 1024;
byte[] buffer = pool.RentBuffer(MB, logger);
pool.ReturnBuffer(buffer, logger);

byte[] buffer2 = pool.RentBuffer(MB, logger);
Assert.AreSame(buffer, buffer2, "Expected recycling of 1 MB buffer.");

// Verify buffer that buffer not power of 2 is not pooled.
buffer = pool.RentBuffer(MB - 1, logger);
pool.ReturnBuffer(buffer, logger);

buffer2 = pool.RentBuffer(MB - 1, logger);
Assert.AreNotSame(buffer, buffer2, "Did not expect recycling of 1 MB - 1 byte buffer.");

// Run through a bunch of random buffer sizes and make sure we always get a buffer of the right size.
int seed = Environment.TickCount;
Console.WriteLine($"Random seed: {seed}");
Random random = new Random(seed);

for (int i = 0; i < 1000; i++)
{
int size = random.Next(0, 2 * MB);
buffer = pool.RentBuffer((uint)size, logger);
Assert.AreEqual(size, buffer.Length, "Wrong buffer size.");
pool.ReturnBuffer(buffer, logger);
}
}
}
}
108 changes: 30 additions & 78 deletions DokanNet.Tests/DirectoryInfoTest.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Linq;
Expand Down Expand Up @@ -338,13 +339,12 @@ public void Delete_WhereRecurseIsFalse_CallsApiCorrectly()
#endif
}

[TestMethod, TestCategory(TestCategories.Success)]
[DeploymentItem("DirectoryInfoTests.Configuration.xml")]
[DataSource("Microsoft.VisualStudio.TestTools.DataSource.XML", "|DataDirectory|\\DirectoryInfoTests.Configuration.xml", "ConfigFindFiles", DataAccessMethod.Sequential)]
public void Delete_WhereRecurseIsTrueAndDirectoryIsNonempty_CallsApiCorrectly()
{
var supportsPatternSearch = bool.Parse((string) TestContext.DataRow["SupportsPatternSearch"]);
public static IEnumerable<object[]> ConfigFindFilesData
=> new object[][] { new object[] { true }, new object[] { false } };

[DataTestMethod, TestCategory(TestCategories.Success), DynamicData(nameof(ConfigFindFilesData))]
public void Delete_WhereRecurseIsTrueAndDirectoryIsNonempty_CallsApiCorrectly(bool supportsPatternSearch)
{
var fixture = DokanOperationsFixture.Instance;

string path = fixture.DirectoryName.AsRootedPath(),
Expand Down Expand Up @@ -397,13 +397,9 @@ public void Delete_WhereRecurseIsTrueAndDirectoryIsNonempty_CallsApiCorrectly()
#endif
}

[TestMethod, TestCategory(TestCategories.Success)]
[DeploymentItem("DirectoryInfoTests.Configuration.xml")]
[DataSource("Microsoft.VisualStudio.TestTools.DataSource.XML", "|DataDirectory|\\DirectoryInfoTests.Configuration.xml", "ConfigFindFiles", DataAccessMethod.Sequential)]
public void Delete_WhereRecurseIsTrueAndDirectoryIsEmpty_CallsApiCorrectly()
[DataTestMethod, TestCategory(TestCategories.Success), DynamicData(nameof(ConfigFindFilesData))]
public void Delete_WhereRecurseIsTrueAndDirectoryIsEmpty_CallsApiCorrectly(bool supportsPatternSearch)
{
var supportsPatternSearch = bool.Parse((string) TestContext.DataRow["SupportsPatternSearch"]);

var fixture = DokanOperationsFixture.Instance;

var path = fixture.DirectoryName.AsRootedPath();
Expand Down Expand Up @@ -515,13 +511,9 @@ public void GetDirectories_OnRootDirectory_WithoutPatternSearch_CallsApiCorrectl
}

[SuppressMessage("Microsoft.Naming", "CA1702:CompoundWordsShouldBeCasedCorrectly", MessageId = "SubDirectory")]
[TestMethod, TestCategory(TestCategories.Success)]
[DeploymentItem("DirectoryInfoTests.Configuration.xml")]
[DataSource("Microsoft.VisualStudio.TestTools.DataSource.XML", "|DataDirectory|\\DirectoryInfoTests.Configuration.xml", "ConfigFindFiles", DataAccessMethod.Sequential)]
public void GetDirectories_OnSubDirectory_CallsApiCorrectly()
[DataTestMethod, TestCategory(TestCategories.Success), DynamicData(nameof(ConfigFindFilesData))]
public void GetDirectories_OnSubDirectory_CallsApiCorrectly(bool supportsPatternSearch)
{
var supportsPatternSearch = bool.Parse((string) TestContext.DataRow["SupportsPatternSearch"]);

var fixture = DokanOperationsFixture.Instance;

var path = fixture.DirectoryName.AsRootedPath();
Expand Down Expand Up @@ -557,13 +549,9 @@ public void GetDirectories_OnSubDirectory_CallsApiCorrectly()
#endif
}

[TestMethod, TestCategory(TestCategories.Success)]
[DeploymentItem("DirectoryInfoTests.Configuration.xml")]
[DataSource("Microsoft.VisualStudio.TestTools.DataSource.XML", "|DataDirectory|\\DirectoryInfoTests.Configuration.xml", "ConfigFindFiles", DataAccessMethod.Sequential)]
public void GetDirectoriesWithFilter_OnRootDirectory_CallsApiCorrectly()
[DataTestMethod, TestCategory(TestCategories.Success), DynamicData(nameof(ConfigFindFilesData))]
public void GetDirectoriesWithFilter_OnRootDirectory_CallsApiCorrectly(bool supportsPatternSearch)
{
var supportsPatternSearch = bool.Parse((string) TestContext.DataRow["SupportsPatternSearch"]);

var fixture = DokanOperationsFixture.Instance;

var path = DokanOperationsFixture.RootName;
Expand Down Expand Up @@ -602,13 +590,9 @@ public void GetDirectoriesWithFilter_OnRootDirectory_CallsApiCorrectly()
#endif
}

[TestMethod, TestCategory(TestCategories.Success)]
[DeploymentItem("DirectoryInfoTests.Configuration.xml")]
[DataSource("Microsoft.VisualStudio.TestTools.DataSource.XML", "|DataDirectory|\\DirectoryInfoTests.Configuration.xml", "ConfigFindFiles", DataAccessMethod.Sequential)]
public void GetFiles_OnRootDirectory_CallsApiCorrectly()
[DataTestMethod, TestCategory(TestCategories.Success), DynamicData(nameof(ConfigFindFilesData))]
public void GetFiles_OnRootDirectory_CallsApiCorrectly(bool supportsPatternSearch)
{
var supportsPatternSearch = bool.Parse((string) TestContext.DataRow["SupportsPatternSearch"]);

var fixture = DokanOperationsFixture.Instance;

var path = DokanOperationsFixture.RootName;
Expand Down Expand Up @@ -645,13 +629,9 @@ public void GetFiles_OnRootDirectory_CallsApiCorrectly()
}

[SuppressMessage("Microsoft.Naming", "CA1702:CompoundWordsShouldBeCasedCorrectly", MessageId = "SubDirectory")]
[TestMethod, TestCategory(TestCategories.Success)]
[DeploymentItem("DirectoryInfoTests.Configuration.xml")]
[DataSource("Microsoft.VisualStudio.TestTools.DataSource.XML", "|DataDirectory|\\DirectoryInfoTests.Configuration.xml", "ConfigFindFiles", DataAccessMethod.Sequential)]
public void GetFiles_OnSubDirectory_CallsApiCorrectly()
[DataTestMethod, TestCategory(TestCategories.Success), DynamicData(nameof(ConfigFindFilesData))]
public void GetFiles_OnSubDirectory_CallsApiCorrectly(bool supportsPatternSearch)
{
var supportsPatternSearch = bool.Parse((string) TestContext.DataRow["SupportsPatternSearch"]);

var fixture = DokanOperationsFixture.Instance;

var path = fixture.DirectoryName.AsRootedPath();
Expand Down Expand Up @@ -687,13 +667,9 @@ public void GetFiles_OnSubDirectory_CallsApiCorrectly()
#endif
}

[TestMethod, TestCategory(TestCategories.Success)]
[DeploymentItem("DirectoryInfoTests.Configuration.xml")]
[DataSource("Microsoft.VisualStudio.TestTools.DataSource.XML", "|DataDirectory|\\DirectoryInfoTests.Configuration.xml", "ConfigFindFiles", DataAccessMethod.Sequential)]
public void GetFilesWithFilter_OnRootDirectory_CallsApiCorrectly()
[DataTestMethod, TestCategory(TestCategories.Success), DynamicData(nameof(ConfigFindFilesData))]
public void GetFilesWithFilter_OnRootDirectory_CallsApiCorrectly(bool supportsPatternSearch)
{
var supportsPatternSearch = bool.Parse((string) TestContext.DataRow["SupportsPatternSearch"]);

var fixture = DokanOperationsFixture.Instance;

var path = DokanOperationsFixture.RootName;
Expand Down Expand Up @@ -732,13 +708,9 @@ public void GetFilesWithFilter_OnRootDirectory_CallsApiCorrectly()
}

[SuppressMessage("Microsoft.Naming", "CA1702:CompoundWordsShouldBeCasedCorrectly", MessageId = "SubDirectory")]
[TestMethod, TestCategory(TestCategories.Success)]
[DeploymentItem("DirectoryInfoTests.Configuration.xml")]
[DataSource("Microsoft.VisualStudio.TestTools.DataSource.XML", "|DataDirectory|\\DirectoryInfoTests.Configuration.xml", "ConfigFindFiles", DataAccessMethod.Sequential)]
public void GetFiles_UnknownDates_CallsApiCorrectly()
[DataTestMethod, TestCategory(TestCategories.Success), DynamicData(nameof(ConfigFindFilesData))]
public void GetFiles_UnknownDates_CallsApiCorrectly(bool supportsPatternSearch)
{
var supportsPatternSearch = bool.Parse((string) TestContext.DataRow["SupportsPatternSearch"]);

var fixture = DokanOperationsFixture.Instance;

var path = fixture.DirectoryName.AsRootedPath();
Expand Down Expand Up @@ -795,13 +767,9 @@ public void GetFiles_UnknownDates_CallsApiCorrectly()
#endif
}

[TestMethod, TestCategory(TestCategories.Success)]
[DeploymentItem("DirectoryInfoTests.Configuration.xml")]
[DataSource("Microsoft.VisualStudio.TestTools.DataSource.XML", "|DataDirectory|\\DirectoryInfoTests.Configuration.xml", "ConfigFindFiles", DataAccessMethod.Sequential)]
public void GetFileSystemInfos_OnRootDirectory_CallsApiCorrectly()
[DataTestMethod, TestCategory(TestCategories.Success), DynamicData(nameof(ConfigFindFilesData))]
public void GetFileSystemInfos_OnRootDirectory_CallsApiCorrectly(bool supportsPatternSearch)
{
var supportsPatternSearch = bool.Parse((string) TestContext.DataRow["SupportsPatternSearch"]);

var fixture = DokanOperationsFixture.Instance;

var path = DokanOperationsFixture.RootName;
Expand Down Expand Up @@ -836,13 +804,9 @@ public void GetFileSystemInfos_OnRootDirectory_CallsApiCorrectly()
}

[SuppressMessage("Microsoft.Naming", "CA1702:CompoundWordsShouldBeCasedCorrectly", MessageId = "SubDirectory")]
[TestMethod, TestCategory(TestCategories.Success)]
[DeploymentItem("DirectoryInfoTests.Configuration.xml")]
[DataSource("Microsoft.VisualStudio.TestTools.DataSource.XML", "|DataDirectory|\\DirectoryInfoTests.Configuration.xml", "ConfigFindFiles", DataAccessMethod.Sequential)]
public void GetFileSystemInfos_OnSubDirectory_CallsApiCorrectly()
[DataTestMethod, TestCategory(TestCategories.Success), DynamicData(nameof(ConfigFindFilesData))]
public void GetFileSystemInfos_OnSubDirectory_CallsApiCorrectly(bool supportsPatternSearch)
{
var supportsPatternSearch = bool.Parse((string) TestContext.DataRow["SupportsPatternSearch"]);

var fixture = DokanOperationsFixture.Instance;

var path = fixture.DirectoryName.AsRootedPath();
Expand Down Expand Up @@ -876,13 +840,9 @@ public void GetFileSystemInfos_OnSubDirectory_CallsApiCorrectly()
#endif
}

[TestMethod, TestCategory(TestCategories.Success)]
[DeploymentItem("DirectoryInfoTests.Configuration.xml")]
[DataSource("Microsoft.VisualStudio.TestTools.DataSource.XML", "|DataDirectory|\\DirectoryInfoTests.Configuration.xml", "ConfigFindFiles", DataAccessMethod.Sequential)]
public void GetFileSystemInfosWithFilter_OnRootDirectory_CallsApiCorrectly()
[DataTestMethod, TestCategory(TestCategories.Success), DynamicData(nameof(ConfigFindFilesData))]
public void GetFileSystemInfosWithFilter_OnRootDirectory_CallsApiCorrectly(bool supportsPatternSearch)
{
var supportsPatternSearch = bool.Parse((string) TestContext.DataRow["SupportsPatternSearch"]);

var fixture = DokanOperationsFixture.Instance;

var path = DokanOperationsFixture.RootName;
Expand Down Expand Up @@ -918,13 +878,9 @@ public void GetFileSystemInfosWithFilter_OnRootDirectory_CallsApiCorrectly()
#endif
}

[TestMethod, TestCategory(TestCategories.Success)]
[DeploymentItem("DirectoryInfoTests.Configuration.xml")]
[DataSource("Microsoft.VisualStudio.TestTools.DataSource.XML", "|DataDirectory|\\DirectoryInfoTests.Configuration.xml", "ConfigFindFiles", DataAccessMethod.Sequential)]
public void GetFileSystemInfos_OnRootDirectory_WhereSearchOptionIsAllDirectories_CallsApiCorrectly()
[DataTestMethod, TestCategory(TestCategories.Success), DynamicData(nameof(ConfigFindFilesData))]
public void GetFileSystemInfos_OnRootDirectory_WhereSearchOptionIsAllDirectories_CallsApiCorrectly(bool supportsPatternSearch)
{
var supportsPatternSearch = bool.Parse((string) TestContext.DataRow["SupportsPatternSearch"]);

var fixture = DokanOperationsFixture.Instance;

var pathsAndItems = new[]
Expand Down Expand Up @@ -1075,13 +1031,9 @@ public void MoveTo_WhereTargetExists_Throws()
#endif
}

[TestMethod, TestCategory(TestCategories.Success)]
[DeploymentItem("DirectoryInfoTests.Configuration.xml")]
[DataSource("Microsoft.VisualStudio.TestTools.DataSource.XML", "|DataDirectory|\\DirectoryInfoTests.Configuration.xml", "ConfigFindFiles", DataAccessMethod.Sequential)]
public void SetAccessControl_CallsApiCorrectly()
[DataTestMethod, TestCategory(TestCategories.Success), DynamicData(nameof(ConfigFindFilesData))]
public void SetAccessControl_CallsApiCorrectly(bool supportsPatternSearch)
{
var supportsPatternSearch = bool.Parse((string) TestContext.DataRow["SupportsPatternSearch"]);

var fixture = DokanOperationsFixture.Instance;

var path = fixture.DirectoryName;
Expand Down
9 changes: 0 additions & 9 deletions DokanNet.Tests/DirectoryInfoTests.Configuration.xml

This file was deleted.

13 changes: 6 additions & 7 deletions DokanNet.Tests/DokanNet.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -20,24 +20,23 @@
<!--Add the Target Framework to the output file names. -->
<AssemblyName>$(MSBuildProjectName).$(TargetFramework)</AssemblyName>
<CLSCompliant>True</CLSCompliant>
<!-- We need to sign the test assembly to use it in InternalsVisibleTo for DokanNet.dll. -->
<SignAssembly>true</SignAssembly>
<AssemblyOriginatorKeyFile>..\DokanNet\Dokan.snk</AssemblyOriginatorKeyFile>
</PropertyGroup>

<ItemGroup>
<Content Include="DirectoryInfoTests.Configuration.xml">
<DependentUpon>DirectoryInfoTest.cs</DependentUpon>
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</Content>
<Content Include="OverlappedTests.Configuration.xml">
<DependentUpon>OverlappedTests.cs</DependentUpon>
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</Content>
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.0.0" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.8.0" />
<PackageReference Include="Moq" Version="4.7.1" />
<PackageReference Include="MSTest.TestAdapter" Version="1.1.12" />
<PackageReference Include="MSTest.TestFramework" Version="1.1.11" />
<PackageReference Include="MSTest.TestAdapter" Version="1.3.2" />
<PackageReference Include="MSTest.TestFramework" Version="1.3.2" />
</ItemGroup>

<ItemGroup>
Expand Down
Loading

0 comments on commit b243cb2

Please sign in to comment.