New reader #506
Open
New reader #506
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Copilot finished reviewing on behalf of
Hirogen
December 2, 2025 21:35
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request introduces a significant modernization effort focused on performance optimization through span-based APIs and new reader implementations, while laying the groundwork for future improvements. The PR adds extensive documentation, new interfaces for zero-allocation log processing, and two new stream reader implementations (Channel-based and Pipeline-based), though the Pipeline implementation is incomplete. The changes also include API refinements, code modernization, and comprehensive test coverage for new IPC functionality.
Key Changes:
- New span-based interfaces (
ILogLineSpan,ILogLineSpanColumnizer,ILogStreamReaderSpan) to enable allocation-free log line processing - Reader architecture refactoring with new
ReaderTypeenum and factory pattern for selecting between Legacy, System, Channel, and Pipeline readers (though the enum definition and Pipeline implementation are incomplete) - Comprehensive documentation in
PIPELINES_IMPLEMENTATION_STRATEGY.mddetailing the implementation approach for a high-performance Pipeline-based reader
Reviewed changes
Copilot reviewed 18 out of 20 changed files in this pull request and generated 26 comments.
Show a summary per file
| File | Description |
|---|---|
| src/docs/performance/PIPELINES_IMPLEMENTATION_STRATEGY.md | Adds comprehensive 385-line implementation strategy document for Pipeline-based stream reader with architecture details, performance expectations, and integration guidance |
| src/LogExpert.UI/Dialogs/SettingsDialog.Designer.cs | Reduces minimum line length from 20,000 to 1,000 characters, providing more flexibility but potentially allowing problematic configurations |
| src/LogExpert.UI/Controls/LogWindow/LogWindow.cs | Modernizes regex usage by converting to source-generated regexes using [GeneratedRegex] attribute for improved performance |
| src/LogExpert.Tests/IPC/ActiveWindowTrackingTests.cs | Adds comprehensive 312-line test suite for active window tracking functionality with scenario, edge case, and integration tests |
| src/LogExpert.Tests/ColumnizerPickerTest.cs | Minor formatting cleanup adding blank lines for improved readability in test LogLine implementation |
| src/LogExpert.Core/Interface/ILogStreamReaderSpan.cs | Introduces new interface for span-based stream reading with memory management methods (marked internal, lacks documentation) |
| src/LogExpert.Core/Interface/ILogStreamReader.cs | Code formatting cleanup removing BOM and normalizing whitespace |
| src/LogExpert.Core/Classes/Log/PositionAwareStreamReaderPipeline.cs | Adds skeleton implementation of Pipeline-based reader with most methods throwing NotImplementedException - incomplete and non-functional |
| src/LogExpert.Core/Classes/Log/PositionAwareStreamReaderChannel.cs | Implements new Channel-based asynchronous log reader with pooled buffers and backpressure handling, now the default reader type |
| src/LogExpert.Core/Classes/Log/LogfileReader.cs | Refactors reader selection to use ReaderType enum with switch expression, removes LogLine record, hardcodes Channel reader as default |
| src/LogExpert.Core/Classes/Log/LogBuffer.cs | Modernizes code with collection expressions, improved formatting, and refactored conditional logic |
| src/ColumnizerLib/ITextValue.cs | Marks interface as obsolete and adds extension methods for backward compatibility |
| src/ColumnizerLib/ILogLineSpanColumnizer.cs | Introduces new interface for span-based columnizer operations to reduce string allocations |
| src/ColumnizerLib/ILogLineSpan.cs | Defines ILogLineSpan interface and LogLineSpan ref struct (has compilation errors - ref struct cannot implement interface) |
| src/ColumnizerLib/ILogLine.cs | Adds LogLine readonly struct implementation to replace previous record-based approach |
| src/ColumnizerLib/IInitColumnizer.cs | Removes unused using statements and fixes trailing whitespace in documentation comments |
| src/ColumnizerLib/IFileSystemCallback.cs | Removes unused using statements and normalizes method formatting |
| src/ColumnizerLib/IColumnizerPriority.cs | Removes unused using statements and normalizes method formatting |
| src/ColumnizerLib/IColumnizerConfigurator.cs | Fixes trailing whitespace in documentation comments and normalizes method formatting |
| src/ColumnizerLib/IColumnizedLogLine.cs | Removes unused using statements and extra blank line |
Files not reviewed (1)
- src/LogExpert.UI/Dialogs/SettingsDialog.Designer.cs: Language not supported
src/LogExpert.Core/Classes/Log/PositionAwareStreamReaderPipeline.cs
Outdated
Show resolved
Hide resolved
src/LogExpert.Core/Classes/Log/PositionAwareStreamReaderPipeline.cs
Outdated
Show resolved
Hide resolved
src/LogExpert.Core/Classes/Log/PositionAwareStreamReaderChannel.cs
Outdated
Show resolved
Hide resolved
src/LogExpert.Core/Classes/Log/PositionAwareStreamReaderChannel.cs
Outdated
Show resolved
Hide resolved
| _updateIntervalMs = updateIntervalMs; | ||
|
|
||
| // Start timer | ||
| _timer = new Timer(ProcessQueue, null, updateIntervalMs, updateIntervalMs); |
Comment on lines
+82
to
+86
| catch (Exception ex) | ||
| { | ||
| // Log but don't crash | ||
| System.Diagnostics.Debug.WriteLine($"Error in progress callback: {ex.Message}"); | ||
| } |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
added 2 commits
December 6, 2025 10:53
ReadOnlyMemory Interface for testing
…into new_reader
| private LineSegment? _currentSegment; | ||
|
|
||
| private PipeReader _pipeReader; | ||
| private CancellationTokenSource _cts; |
Comment on lines
+265
to
+268
| catch (Exception) | ||
| { | ||
| // Ignore errors during completion | ||
| } |
Comment on lines
+324
to
+328
| catch (Exception ex) | ||
| { | ||
| // Store exception to rethrow in ReadLine | ||
| Volatile.Write(ref _producerException, ex); | ||
| } |
| private LineSegment? _currentSegment; | ||
|
|
||
| private PipeReader _pipeReader; | ||
| private CancellationTokenSource _cts; |
Comment on lines
+290
to
+293
| catch (Exception) | ||
| { | ||
| // Ignore errors during completion | ||
| } |
Comment on lines
+349
to
+353
| catch (Exception ex) | ||
| { | ||
| // Store exception to rethrow in ReadLine | ||
| Volatile.Write(ref _producerException, ex); | ||
| } |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request introduces several major improvements and refactorings to the log reading and columnizer infrastructure, with a focus on performance, extensibility, and benchmarking. The most significant changes include the addition of span-based interfaces and types for log lines, new benchmarking infrastructure using BenchmarkDotNet, and updates to configuration and preferences for reader selection. Deprecated interfaces and settings are also marked for future removal.
Performance and Span-based Infrastructure
ILogLineSpan,LogLineSpan, andILogLineSpanColumnizer, enabling efficient, allocation-free parsing and timestamp extraction of log lines. These changes allow columnizers and readers to operate directly on memory spans, improving performance for large log files. (src/ColumnizerLib/ILogLineSpan.cs,src/ColumnizerLib/ILogLineSpanColumnizer.cs) [1] [2]ILogStreamReaderSpaninterface for span-based log stream reading, supporting memory-efficient line reading and management. (src/LogExpert.Core/Interface/ILogStreamReaderSpan.cs)Benchmarking and Testing
LogExpert.Benchmarksproject using BenchmarkDotNet, with benchmarks for different stream reader implementations (Legacy,System,Pipeline). This includes project configuration files and a comprehensive benchmark suite for measuring performance across different file sizes and encodings. (src/LogExpert.Benchmarks/LogExpert.Benchmarks.csproj,src/LogExpert.Benchmarks/StreamReaderBenchmarks.cs,src/LogExpert.Benchmarks/Directory.Build.props,src/LogExpert.Benchmarks/Directory.Build.targets,src/Directory.Packages.props) [1] [2] [3] [4] [5]Configuration and Preferences
ReaderTypeenum to select between pipeline, legacy, and system reader implementations, and updated thePreferencesclass to use this new setting. The obsoleteUseLegacyReadersetting is now marked for removal and ignored in serialization. (src/LogExpert.Core/Config/Preferences.cs,src/LogExpert.Core/Enums/ReaderType.cs) [1] [2]Codebase Modernization and Cleanup
ITextValueinterface, added[Obsolete]attributes and extension methods to guide migration to direct property access, and removed unnecessaryusingstatements from several interface files for cleaner code. (src/ColumnizerLib/ITextValue.cs,src/ColumnizerLib/IColumnizedLogLine.cs,src/ColumnizerLib/IColumnizerPriority.cs,src/ColumnizerLib/IFileSystemCallback.cs,src/ColumnizerLib/IInitColumnizer.cs) (F8191294R1, [1] [2] [3] [4] [5]Log Buffer and Progress Reporting Enhancements
LogBufferwith modern C# syntax and clarified constructor usage, and introducedBatchedProgressReporterto efficiently batch progress updates and reduce UI thread overhead during file loading. (src/LogExpert.Core/Classes/Log/LogBuffer.cs,src/LogExpert.Core/Classes/Log/BatchedProgressReporter.cs) [1] [2] [3] [4]These changes collectively improve performance, maintainability, and extensibility of the log reading and columnizing system, while laying the groundwork for future enhancements.
Benchmark Results
Intel Core Ultra 5 135U Results
AMD Ryzen 9 5900X Results