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

EmptyDirectoryConstraint doesn't need to enumerate entire directory contents #3622

Closed
stevenaw opened this issue Aug 17, 2020 · 0 comments · Fixed by #3623
Closed

EmptyDirectoryConstraint doesn't need to enumerate entire directory contents #3622

stevenaw opened this issue Aug 17, 2020 · 0 comments · Fixed by #3623
Milestone

Comments

@stevenaw
Copy link
Member

EmptyDirectoryConstraint currently tries to read in and instantiate all content in a directory in order to determine if the directory is empty or not empty.

https://github.com/nunit/nunit/blob/master/src/NUnitFramework/framework/Constraints/EmptyDirectoryConstraint.cs#L55-L57

An alternative could be to use one call to EnumerateFileSystemInfos().Any() to detect existence of either files or subdirectories without instantiating a whole array of them. Unfortunately, this API isn't available in .NET 35, so some sort of fallback would be required to go this route. A fallback could be either the currently implementation, or using GetFileSystemInfos().Any(). Example:

- files = dirInfo.GetFiles().Length;
- subdirs = dirInfo.GetDirectories().Length;
- bool hasSucceeded = files == 0 && subdirs == 0;

+ bool hasSucceeded;
+ #if !NET35
+           hasSucceeded = !dirInfo.EnumerateFileSystemInfos().Any();
+ #else
+           hasSucceeded = !dirInfo.GetFileSystemInfos().Any();
+ #endif

I've done a quick verification via BenchmarkDotNet, and it seems promising (N = number of files, N/2 = number of subdirectories)

|           Method |    N |        Mean |        Error |    StdDev |    Gen 0 |   Gen 1 | Gen 2 | Allocated |
|----------------- |----- |------------:|-------------:|----------:|---------:|--------:|------:|----------:|
| IsEmpty_Original |    0 |    44.18 us |    11.016 us |  0.604 us |   0.1221 |       - |     - |     592 B |
|      IsEmpty_New |    0 |    21.05 us |     2.992 us |  0.164 us |   0.0610 |       - |     - |     296 B |
| IsEmpty_Original |  100 |   115.95 us |     3.041 us |  0.167 us |  11.9629 |  0.1221 |     - |   50216 B |
|      IsEmpty_New |  100 |    61.05 us |   153.207 us |  8.398 us |   0.1221 |       - |     - |     600 B |
| IsEmpty_Original | 1024 | 1,447.30 us | 1,445.389 us | 79.227 us | 105.4688 | 33.2031 |     - |  508424 B |
|      IsEmpty_New | 1024 |    79.75 us |    31.172 us |  1.709 us |   0.1221 |       - |     - |     600 B |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants