Change `FSDirectory.Open()` defaults to match .NET-specific platform and performance concerns
- Dominant language
- C#
- Stars
- 2.4k
- Forks
- 658
- Avg merge
- 3d 5h
- Merged PRs (30d)
- 9
Description
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Task description
The `FSDirectory.Open()` method contains some business logic to select the best `FSDirectory` implementation for the current platform.
https://github.com/apache/lucenenet/blob/5976abd955fef2dcab855f7632782680b2c69b55/src/Lucene.Net/Store/FSDirectory.cs#L184-L203
However, this business logic is based on what is the best option in Java. The 3 directory types may have very different performance characteristics in .NET due to a fundamentally different approach to providing random access and memory-mapped access to files.
We need to do some investigation based on performance profiles which directory option correctly maps to:
- Specific operating systems
- Runtime architecture (x64/x86/ARM64, etc.)
- Target Framework
> Note that this is currently blocked by #1267. We should do this investigation based on the new implementation.
## AOT Limitations
After researching AOT limitations (see #1278), we need to ensure that the `MMapDirectory` is never selected as the default option where dynamic code is not supported. The canonical way to do that is to check:
```c#
if (!System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported)
{
// AOT is likely in use and dynamic features are disabled.
// Fallback to standard FileStream IO or handle gracefully.
}
```
Although, we should add that check to the `Constants` class so it can be shared project-wide. We could fall back to `NIOFSDirectory` instead of `SimpleFSDirectory` if it performs better, but it will take some experimentation to make that determination.
> Note that the API is not supported on .NET Framework or .NET Standard 2.0, but we can build an internal conditionally compiled stub that is hard coded to `true` for that case as was [already done in J2N](https://github.com/NightOwl888/J2N/blob/4209ca0ca7a599ce2e1fc9fa346b13eef9578437/src/J2N/compatibility/RuntimeFeature.cs).
## .NET Framework/.NET Standard 2.0 Limitations
The [System.IO.RandomAccess](https://learn.microsoft.com/en-us/dotnet/api/system.io.randomaccess?view=net-10.0) class did not exist before .NET 6. As a result the `NIOFSDirectory` is not currently a good candidate for being the default on anything other than .NET Core.
There has been some discussion about possibly making a port of `System.IO.RandomAccess` (or even a cut down version to patch our needs), but until it is actually done we should not select this directory as the default on older TFMs than .NET Core. Note that `NIOFSDirecotry` only uses the [`Read(SafeFileHandle, Span, long)`](https://learn.microsoft.com/en-us/dotnet/api/system.io.randomaccess.read?view=net-10.0#system-io-randomaccess-read(microsoft-win32-safehandles-safefilehandle-system-span((system-byte))-system-int64)) overload.
## macOS
In Java, the `NIOFSDirectory` was chosen over `MMapDirectory` on macOS. I suspect that once #1278 is completed `MMapDirectory` will probably be the best option instead, but it will require testing to confirm.
-----------------------------
I suspect there will be other concerns down the road on which directory is best on a given platform, but we can deal with those concerns as they become known. It might not hurt to have AI audit our configuration and make some suggestions, though.
Contributor guide
Research direction
Start with the FSDirectory.Open() logic in src/Lucene.Net/Store/FSDirectory.cs at the linked lines, then review the Constants class and the blocked work in #1267 and #1278. Run performance profiles across the listed operating systems, architectures, and target frameworks, including AOT; done means the defaults are validated and MMapDirectory is not selected where dynamic code is unsupported.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- operating-systems, performance
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100