Skip to content

Commit 3681fe1

Browse files
paulirwinCopilotclaude
authored
SWEEP: Avoid unnecessary FileInfo/DirectoryInfo allocations, #832 (#1060)
* Avoid unnecessary FileInfo/DirectoryInfo allocations, #832 * Remove extra semicolon in WriteLineDocTask.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Enable NRT on StopAnalyzer and StopwordAnalyzerBase * Add comments about parameter normalization * Revert FileInfo change in CompoundFileExtractor (while leaving null check) * Add string-based constructor tests for FilesystemResourceLoader Added comprehensive tests for the new string-based constructors in FilesystemResourceLoader: - TestBaseDirWithString: Tests string path instead of DirectoryInfo - TestDelegationWithString: Tests delegation with string base directory - TestRelativePathsWithString: Tests various relative path scenarios - TestRelativePathsWithStringAndDelegation: Tests relative paths with delegation fallback These tests ensure the string-based constructors work correctly alongside the existing DirectoryInfo-based ones. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * Add comprehensive tests for string-based overloads (Issue #832) Added tests for the new string-based constructors and methods that avoid unnecessary FileInfo/DirectoryInfo allocations: StopAnalyzer Tests: - TestStopAnalyzerWithStringFileName: Tests string-based constructor with absolute paths - TestStopAnalyzerWithRelativePath: Tests string-based constructor with relative paths - TestLoadStopwordSetWithStringPath: Tests StopwordAnalyzerBase.LoadStopwordSet(string, LuceneVersion) FilesystemResourceLoader Tests: - TestNestedRelativePathsWithString: Tests nested relative paths with string-based constructors FileUtils Tests: - TestFullyDeleteWithStringPath: Tests FullyDelete(string) with absolute paths - TestFullyDeleteNonExistentStringPath: Tests behavior with non-existent paths - TestFullyDeleteWithDirectoryInfo: Tests original DirectoryInfo overload - TestFullyDeleteEmptyDirectoryWithStringPath: Tests with empty directories - TestFullyDeleteWithRelativeStringPath: Tests with relative paths All tests ensure the new string-based overloads work correctly with both absolute and relative paths. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * Add SystemEnvironment lock for thread-safe Environment.CurrentDirectory changes Created SystemEnvironment helper class in TestFramework to prevent race conditions when tests that modify Environment.CurrentDirectory run in parallel. This addresses potential issues identified during the string-based overload additions (Issue #832). Changes: - Added SystemEnvironment.cs with thread-safe lock mechanism using SemaphoreSlim - Provides WithCurrentDirectory methods that automatically restore the original directory - Updated all tests that change Environment.CurrentDirectory to use SystemEnvironment: * TestStopAnalyzer.TestStopAnalyzerWithRelativePath * TestFilesystemResourceLoader (multiple methods) * TestFileUtils.TestFullyDeleteWithRelativeStringPath * TestStreamUtils (input/output stream tests) * TestIOUtils.TestGetDecodingReaderWithRelativePath * TestWriteEnwikiLineDocTask.TestCategoriesLineFileWithRelativePath This ensures tests can safely run in parallel without interfering with each other's current directory state. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix whitespace: Add final newlines to source files 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix Windows test failures: Use platform-aware path separators - Use Path.Combine in test assertions to ensure correct path separators - Remove Unix-style absolute paths starting with slash - Add platform-specific absolute path tests for Windows and Unix 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix path separator issues: Use Path.Combine for platform compatibility - Replace hardcoded path separators with Path.Combine - Ensures correct separators on both Windows and Unix - Fixes test failures on Windows 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent b34c8f6 commit 3681fe1

43 files changed

Lines changed: 1660 additions & 168 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎Lucene.Net.sln.DotSettings‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,9 @@
22
<s:Boolean x:Key="/Default/UserDictionary/Words/=Coord/@EntryIndexedValue">True</s:Boolean>
33
<s:Boolean x:Key="/Default/UserDictionary/Words/=csharpsquid/@EntryIndexedValue">True</s:Boolean>
44
<s:Boolean x:Key="/Default/UserDictionary/Words/=LUCENENET/@EntryIndexedValue">True</s:Boolean>
5+
<s:Boolean x:Key="/Default/UserDictionary/Words/=stopword/@EntryIndexedValue">True</s:Boolean>
6+
<s:Boolean x:Key="/Default/UserDictionary/Words/=stopwords/@EntryIndexedValue">True</s:Boolean>
7+
<s:Boolean x:Key="/Default/UserDictionary/Words/=streamutils/@EntryIndexedValue">True</s:Boolean>
8+
<s:Boolean x:Key="/Default/UserDictionary/Words/=subdir/@EntryIndexedValue">True</s:Boolean>
59
<s:Boolean x:Key="/Default/UserDictionary/Words/=synch/@EntryIndexedValue">True</s:Boolean>
610
<s:Boolean x:Key="/Default/UserDictionary/Words/=testsettings/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary>

‎src/Lucene.Net.Analysis.Common/Analysis/Compound/Hyphenation/HyphenationTree.cs‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -127,9 +127,7 @@ protected virtual string UnpackValues(int k)
127127
/// <param name="filename"> the filename </param>
128128
/// <exception cref="IOException"> In case the parsing fails </exception>
129129
public virtual void LoadPatterns(string filename)
130-
{
131-
LoadPatterns(filename, Encoding.UTF8);
132-
}
130+
=> LoadPatterns(filename, Encoding.UTF8);
133131

134132
/// <summary>
135133
/// Read hyphenation patterns from an XML file.
@@ -149,9 +147,7 @@ public virtual void LoadPatterns(string filename, Encoding encoding)
149147
/// <param name="f"> a <see cref="FileInfo"/> object representing the file </param>
150148
/// <exception cref="IOException"> In case the parsing fails </exception>
151149
public virtual void LoadPatterns(FileInfo f)
152-
{
153-
LoadPatterns(f, Encoding.UTF8);
154-
}
150+
=> LoadPatterns(f.FullName, Encoding.UTF8);
155151

156152
/// <summary>
157153
/// Read hyphenation patterns from an XML file.
@@ -160,20 +156,15 @@ public virtual void LoadPatterns(FileInfo f)
160156
/// <param name="encoding">The character encoding to use</param>
161157
/// <exception cref="IOException"> In case the parsing fails </exception>
162158
public virtual void LoadPatterns(FileInfo f, Encoding encoding)
163-
{
164-
var src = new FileStream(f.FullName, FileMode.Open, FileAccess.Read);
165-
LoadPatterns(src, encoding);
166-
}
159+
=> LoadPatterns(f.FullName, encoding);
167160

168161
/// <summary>
169162
/// Read hyphenation patterns from an XML file.
170163
/// </summary>
171164
/// <param name="source"> <see cref="Stream"/> input source for the file </param>
172165
/// <exception cref="IOException"> In case the parsing fails </exception>
173166
public virtual void LoadPatterns(Stream source)
174-
{
175-
LoadPatterns(source, Encoding.UTF8);
176-
}
167+
=> LoadPatterns(source, Encoding.UTF8);
177168

178169
/// <summary>
179170
/// Read hyphenation patterns from an XML file.

‎src/Lucene.Net.Analysis.Common/Analysis/Compound/Hyphenation/PatternParser.cs‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,7 @@ public virtual IPatternConsumer Consumer
8080
/// <param name="path">The complete file path to be read.</param>
8181
/// <exception cref="IOException"> In case of an exception while parsing </exception>
8282
public virtual void Parse(string path)
83-
{
84-
Parse(path, Encoding.UTF8);
85-
}
83+
=> Parse(path, Encoding.UTF8);
8684

8785
/// <summary>
8886
/// Parses a hyphenation pattern file.
@@ -103,9 +101,7 @@ public virtual void Parse(string path, Encoding encoding)
103101
/// <param name="file"> a <see cref="FileInfo"/> object representing the file </param>
104102
/// <exception cref="IOException"> In case of an exception while parsing </exception>
105103
public virtual void Parse(FileInfo file)
106-
{
107-
Parse(file, Encoding.UTF8);
108-
}
104+
=> Parse(file.FullName, Encoding.UTF8);
109105

110106
/// <summary>
111107
/// Parses a hyphenation pattern file.
@@ -114,12 +110,7 @@ public virtual void Parse(FileInfo file)
114110
/// <param name="encoding">The character encoding to use</param>
115111
/// <exception cref="IOException"> In case of an exception while parsing </exception>
116112
public virtual void Parse(FileInfo file, Encoding encoding)
117-
{
118-
var xmlReaderSettings = GetXmlReaderSettings();
119-
120-
using var src = XmlReader.Create(new StreamReader(file.OpenRead(), encoding), xmlReaderSettings);
121-
Parse(src);
122-
}
113+
=> Parse(file.FullName, encoding);
123114

124115
/// <summary>
125116
/// Parses a hyphenation pattern file.

‎src/Lucene.Net.Analysis.Common/Analysis/Compound/HyphenationCompoundWordTokenFilter.cs‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -131,9 +131,7 @@ public HyphenationCompoundWordTokenFilter(LuceneVersion matchVersion, TokenStrea
131131
/// <returns> An object representing the hyphenation patterns </returns>
132132
/// <exception cref="IOException"> If there is a low-level I/O error. </exception>
133133
public static HyphenationTree GetHyphenationTree(string hyphenationFilename)
134-
{
135-
return GetHyphenationTree(hyphenationFilename, Encoding.UTF8);
136-
}
134+
=> GetHyphenationTree(hyphenationFilename, Encoding.UTF8);
137135

138136
/// <summary>
139137
/// Create a hyphenator tree
@@ -143,9 +141,7 @@ public static HyphenationTree GetHyphenationTree(string hyphenationFilename)
143141
/// <returns> An object representing the hyphenation patterns </returns>
144142
/// <exception cref="IOException"> If there is a low-level I/O error. </exception>
145143
public static HyphenationTree GetHyphenationTree(string hyphenationFilename, Encoding encoding)
146-
{
147-
return GetHyphenationTree(new FileStream(hyphenationFilename, FileMode.Open, FileAccess.Read), encoding);
148-
}
144+
=> GetHyphenationTree(new FileStream(hyphenationFilename, FileMode.Open, FileAccess.Read), encoding);
149145

150146
/// <summary>
151147
/// Create a hyphenator tree
@@ -154,9 +150,7 @@ public static HyphenationTree GetHyphenationTree(string hyphenationFilename, Enc
154150
/// <returns> An object representing the hyphenation patterns </returns>
155151
/// <exception cref="IOException"> If there is a low-level I/O error. </exception>
156152
public static HyphenationTree GetHyphenationTree(FileInfo hyphenationFile)
157-
{
158-
return GetHyphenationTree(hyphenationFile, Encoding.UTF8);
159-
}
153+
=> GetHyphenationTree(hyphenationFile.FullName, Encoding.UTF8);
160154

161155
/// <summary>
162156
/// Create a hyphenator tree
@@ -166,9 +160,7 @@ public static HyphenationTree GetHyphenationTree(FileInfo hyphenationFile)
166160
/// <returns> An object representing the hyphenation patterns </returns>
167161
/// <exception cref="IOException"> If there is a low-level I/O error. </exception>
168162
public static HyphenationTree GetHyphenationTree(FileInfo hyphenationFile, Encoding encoding)
169-
{
170-
return GetHyphenationTree(new FileStream(hyphenationFile.FullName, FileMode.Open, FileAccess.Read), encoding);
171-
}
163+
=> GetHyphenationTree(hyphenationFile.FullName, encoding);
172164

173165
/// <summary>
174166
/// Create a hyphenator tree
@@ -177,9 +169,7 @@ public static HyphenationTree GetHyphenationTree(FileInfo hyphenationFile, Encod
177169
/// <returns> An object representing the hyphenation patterns </returns>
178170
/// <exception cref="IOException"> If there is a low-level I/O error. </exception>
179171
public static HyphenationTree GetHyphenationTree(Stream hyphenationSource)
180-
{
181-
return GetHyphenationTree(hyphenationSource, Encoding.UTF8);
182-
}
172+
=> GetHyphenationTree(hyphenationSource, Encoding.UTF8);
183173

184174
/// <summary>
185175
/// Create a hyphenator tree

‎src/Lucene.Net.Analysis.Common/Analysis/Core/StopAnalyzer.cs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
using System.Diagnostics.CodeAnalysis;
66
using System.IO;
77

8+
#nullable enable
9+
810
namespace Lucene.Net.Analysis.Core
911
{
1012
/*
@@ -77,6 +79,19 @@ public StopAnalyzer(LuceneVersion matchVersion, CharArraySet stopWords)
7779
{
7880
}
7981

82+
/// <summary>
83+
/// Builds an analyzer with the stop words from the given file. </summary>
84+
/// <seealso cref="WordlistLoader.GetWordSet(TextReader, LuceneVersion)"/>
85+
/// <param name="matchVersion"> See <see cref="LuceneVersion"/> </param>
86+
/// <param name="stopwordsFileName"> File name to load stop words from. The path is not normalized by this method. </param>
87+
/// <remarks>
88+
/// LUCENENET: This overload takes a string file name to avoid allocating a <see cref="FileInfo"/> object.
89+
/// </remarks>
90+
public StopAnalyzer(LuceneVersion matchVersion, string stopwordsFileName)
91+
: this(matchVersion, LoadStopwordSet(stopwordsFileName, matchVersion))
92+
{
93+
}
94+
8095
/// <summary>
8196
/// Builds an analyzer with the stop words from the given file. </summary>
8297
/// <seealso cref="WordlistLoader.GetWordSet(TextReader, LuceneVersion)"/>

‎src/Lucene.Net.Analysis.Common/Analysis/Util/FilesystemResourceLoader.cs‎

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Lucene version compatibility level 4.8.1
22
using System;
33
using System.IO;
4+
#nullable enable
45

56
namespace Lucene.Net.Analysis.Util
67
{
@@ -38,7 +39,7 @@ namespace Lucene.Net.Analysis.Util
3839
/// </summary>
3940
public sealed class FilesystemResourceLoader : IResourceLoader
4041
{
41-
private readonly DirectoryInfo baseDirectory;
42+
private readonly string? baseDirectory; // LUCENENET specific: changed to use string directory name instead of allocating a DirectoryInfo (#832)
4243
private readonly IResourceLoader @delegate;
4344

4445
/// <summary>
@@ -47,7 +48,7 @@ public sealed class FilesystemResourceLoader : IResourceLoader
4748
/// are delegated to context classloader.
4849
/// </summary>
4950
public FilesystemResourceLoader()
50-
: this((DirectoryInfo)null)
51+
: this((string?)null)
5152
{
5253
}
5354

@@ -57,22 +58,53 @@ public FilesystemResourceLoader()
5758
/// Files not found in file system and class lookups are delegated to context
5859
/// classloader.
5960
/// </summary>
60-
public FilesystemResourceLoader(DirectoryInfo baseDirectory)
61+
/// <param name="baseDirectory">The base directory path. The path is not normalized by this method.</param>
62+
/// <remarks>
63+
/// LUCENENET: This overload takes a string to avoid allocating a <see cref="DirectoryInfo"/> object.
64+
/// </remarks>
65+
public FilesystemResourceLoader(string? baseDirectory)
6166
: this(baseDirectory, new ClasspathResourceLoader(typeof(FilesystemResourceLoader)))
6267
{
6368
}
6469

70+
/// <summary>
71+
/// Creates a resource loader that resolves resources against the given
72+
/// base directory (may be <c>null</c> to refer to CWD).
73+
/// Files not found in file system and class lookups are delegated to context
74+
/// classloader.
75+
/// </summary>
76+
public FilesystemResourceLoader(DirectoryInfo? baseDirectory)
77+
: this(baseDirectory?.FullName, new ClasspathResourceLoader(typeof(FilesystemResourceLoader)))
78+
{
79+
}
80+
81+
/// <summary>
82+
/// Creates a resource loader that resolves resources against the given
83+
/// base directory (may be <c>null</c> to refer to CWD).
84+
/// Files not found in file system and class lookups are delegated
85+
/// to the given delegate <see cref="IResourceLoader"/>.
86+
/// </summary>
87+
public FilesystemResourceLoader(DirectoryInfo? baseDirectory, IResourceLoader @delegate)
88+
: this(baseDirectory?.FullName, @delegate)
89+
{
90+
}
91+
6592
/// <summary>
6693
/// Creates a resource loader that resolves resources against the given
6794
/// base directory (may be <c>null</c> to refer to CWD).
6895
/// Files not found in file system and class lookups are delegated
6996
/// to the given delegate <see cref="IResourceLoader"/>.
7097
/// </summary>
71-
public FilesystemResourceLoader(DirectoryInfo baseDirectory, IResourceLoader @delegate)
98+
/// <param name="baseDirectory">The base directory path. The path is not normalized by this method.</param>
99+
/// <param name="delegate">The delegate resource loader.</param>
100+
/// <remarks>
101+
/// LUCENENET: This overload takes a string to avoid allocating a <see cref="DirectoryInfo"/> object.
102+
/// </remarks>
103+
public FilesystemResourceLoader(string? baseDirectory, IResourceLoader @delegate)
72104
{
73105
// LUCENENET NOTE: If you call DirectoryInfo.Create() it doesn't set the DirectoryInfo.Exists
74106
// flag to true, so we use the Directory object to check the path explicitly.
75-
if (baseDirectory is not null && !Directory.Exists(baseDirectory.FullName))
107+
if (baseDirectory is not null && !Directory.Exists(baseDirectory))
76108
{
77109
throw new ArgumentException("baseDirectory is not a directory or is null");
78110
}
@@ -89,35 +121,35 @@ public Stream OpenResource(string resource)
89121
{
90122
try
91123
{
92-
FileInfo file = null;
124+
string? file = null; // LUCENENET specific: changed to use string file name instead of allocating a FileInfo (#832)
93125

94126
// First try absolute.
95127
if (File.Exists(resource))
96128
{
97-
file = new FileInfo(resource);
129+
file = resource;
98130
}
99131
else
100132
{
101133
// Try as a relative path
102134
var fullPath = System.IO.Path.GetFullPath(resource);
103135
if (File.Exists(fullPath))
104136
{
105-
file = new FileInfo(fullPath);
137+
file = fullPath;
106138
}
107139
else if (baseDirectory != null)
108140
{
109141
// Try to combine with the base directory
110-
string based = System.IO.Path.Combine(baseDirectory.FullName, resource);
142+
string based = System.IO.Path.Combine(baseDirectory, resource);
111143
if (File.Exists(based))
112144
{
113-
file = new FileInfo(based);
145+
file = based;
114146
}
115147
}
116148
}
117149

118150
if (file != null)
119151
{
120-
return file.OpenRead();
152+
return new FileStream(file, FileMode.Open, FileAccess.Read, FileShare.Read);
121153
}
122154

123155
// Fallback on the inner resource loader (this could fail)

‎src/Lucene.Net.Analysis.Common/Analysis/Util/StopwordAnalyzerBase.cs‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
using System.Reflection;
77
using System.Text;
88

9+
#nullable enable
10+
911
namespace Lucene.Net.Analysis.Util
1012
{
1113
/*
@@ -52,7 +54,7 @@ public abstract class StopwordAnalyzerBase : Analyzer
5254
/// the Lucene version for cross version compatibility </param>
5355
/// <param name="stopwords">
5456
/// the analyzer's stopword set </param>
55-
protected StopwordAnalyzerBase(LuceneVersion version, CharArraySet stopwords)
57+
protected StopwordAnalyzerBase(LuceneVersion version, CharArraySet? stopwords)
5658
{
5759
m_matchVersion = version;
5860
// analyzers should use char array set for stopwords!
@@ -88,7 +90,7 @@ protected StopwordAnalyzerBase(LuceneVersion version)
8890
/// if loading the stopwords throws an <see cref="IOException"/> </exception>
8991
protected static CharArraySet LoadStopwordSet(bool ignoreCase, Type aClass, string resource, string comment)
9092
{
91-
TextReader reader = null;
93+
TextReader? reader = null;
9294
try
9395
{
9496
var resourceStream = aClass.FindAndGetManifestResourceStream(resource);
@@ -104,6 +106,33 @@ protected static CharArraySet LoadStopwordSet(bool ignoreCase, Type aClass, stri
104106
}
105107
}
106108

109+
/// <summary>
110+
/// Creates a <see cref="CharArraySet"/> from a file.
111+
/// </summary>
112+
/// <param name="stopwordsFileName">The file name to load stopwords from. The path is not normalized by this method.</param>
113+
/// <param name="matchVersion">
114+
/// the Lucene version for cross version compatibility </param>
115+
/// <returns> a <see cref="CharArraySet"/> containing the distinct stopwords from the given
116+
/// file </returns>
117+
/// <exception cref="IOException">
118+
/// if loading the stopwords throws an <see cref="IOException"/> </exception>
119+
/// <remarks>
120+
/// LUCENENET: This overload takes a string file name to avoid allocating a <see cref="FileInfo"/> object.
121+
/// </remarks>
122+
protected static CharArraySet LoadStopwordSet(string stopwordsFileName, LuceneVersion matchVersion)
123+
{
124+
TextReader? reader = null;
125+
try
126+
{
127+
reader = IOUtils.GetDecodingReader(stopwordsFileName, Encoding.UTF8);
128+
return WordlistLoader.GetWordSet(reader, matchVersion);
129+
}
130+
finally
131+
{
132+
IOUtils.Dispose(reader);
133+
}
134+
}
135+
107136
/// <summary>
108137
/// Creates a <see cref="CharArraySet"/> from a file.
109138
/// </summary>
@@ -118,7 +147,7 @@ protected static CharArraySet LoadStopwordSet(bool ignoreCase, Type aClass, stri
118147
/// if loading the stopwords throws an <see cref="IOException"/> </exception>
119148
protected static CharArraySet LoadStopwordSet(FileInfo stopwords, LuceneVersion matchVersion)
120149
{
121-
TextReader reader = null;
150+
TextReader? reader = null;
122151
try
123152
{
124153
reader = IOUtils.GetDecodingReader(stopwords, Encoding.UTF8);

0 commit comments

Comments
 (0)