alphaleonis / AlphaFS

AlphaFS is a .NET library providing more complete Win32 file system functionality to the .NET platform than the standard System.IO classes.
http://alphafs.alphaleonis.com/
MIT License
566 stars 99 forks source link

Avoid unnecessary allocations from Random construction in FileSystemInfo #364

Closed danmoseley closed 6 years ago

danmoseley commented 7 years ago

We have seen the allocation at private readonly int _random = new Random().Next(0, 19); show up high in allocation profiles when there is heavy use enumerating the file system. Instead of newing up a new Random, one could be reused in a thread static field.

https://github.com/alphaleonis/AlphaFS/blob/afc207bc8ca5de1335e0f224a83dbb2fe81eb8ba/AlphaFS/Filesystem/FileSystemInfo.cs#L70

Yomodo commented 6 years ago

From the MSDN ThreadStaticAttribute Class docs:

Do not specify initial values for fields marked with ThreadStaticAttribute, because such initialization occurs only once, when the class constructor executes, and therefore affects only one thread. If you do not specify an initial value, you can rely on the field being initialized to its default value if it is a value type, or to null if it is a reference type.

From what I understand, I'm not sure if below implementation makes a difference since _random will still be null for every thread.

[ThreadStatic] private static Random _random;
private int _prime;

internal void InitializeCore(KernelTransaction transaction, ...)
{
    if (null == _random)
    _random = new Random();

    _prime = Primes[_random.Next(0, 19)];

    ...

}

danmoseley commented 6 years ago

Ah. Perhaps some form of Lazy then.

alphaleonis commented 6 years ago

Okay, hold on here for a second. Can someone please explain to me why on earth we need a random number generator in FileSystemInfo? Why does GetHashCode() return a random number? One of the invariants of GetHashCode() is that it must always return the same number for two identical objects, and this one does not seem to do that as far as I can tell?

That it to say, it seems quite possible for a.Equals(b) to return true, but for a.GetHashCode() to return a different value from b.GetHashCode()?

Yomodo commented 6 years ago

What you are saying makes perfect sense. I can honestly not remember why GetHashCode was implemented this way.