Merged
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
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); |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
added 2 commits
December 6, 2025 10:53
|
|
||
| for (int i = 0; i < _steps.Count; i++) | ||
| { | ||
| _buffers[i] = new BlockingCollection<object>(100); // Bounded capacity |
| ArgumentNullException.ThrowIfNull(logLine); | ||
| ArgumentNullException.ThrowIfNull(callback); | ||
|
|
||
| return new Log4JLogLine(ReplaceInMemory(logLine.FullLine, SEPARATOR_CHAR, '|'), logLine.Text, logLine.LineNumber); |
| ColumnList.Clear(); | ||
| ColumnSet.Clear(); | ||
|
|
||
| var line = callback.GetLogLineMemory(0); |
| /// <returns>A new <see cref="ILogLineMemory"/> instance containing the clipboard-formatted text of the specified log line.</returns> | ||
| public ILogLineMemory GetLineTextForClipboard (ILogLineMemory logLine, ILogLineMemoryColumnizerCallback callback) | ||
| { | ||
| return new GlassFishLogLine(ReplaceInMemory(logLine.FullLine, SEPARATOR_CHAR, '|'), logLine.Text, logLine.LineNumber); |
| var columns = Column.CreateColumns(COLUMN_COUNT, cLogLine); | ||
| cLogLine.ColumnValues = [.. columns.Select(a => a as IColumn)]; | ||
|
|
||
| var temp = line.FullLine; |
|
|
||
| var columns = Column.CreateColumns(ColumnList.Count, cLogLine); | ||
|
|
||
| columns.Last().FullValue = logLine.FullLine; |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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 a new set of "Memory"-based interfaces and updates the
Columnand related classes to useReadOnlyMemory<char>instead ofstringfor improved performance and flexibility. It also adds new helper methods for memory-based string manipulation and updates unit tests accordingly. The changes are grouped into interface additions/updates, core class refactoring, and unit test adjustments.Interface additions and updates:
IColumnMemory,IColumnizedLogLineMemory,IAutoLogLineMemoryColumnizerCallback,IColumnizerConfiguratorMemory, andIColumnizerPriorityMemoryto supportReadOnlyMemory<char>-based operations and memory-efficient log processing. [1] [2] [3] [4] [5][Obsolete]members and to reference new memory-based types where appropriate, such as inIColumnizedLogLineandIContextMenuEntry. [1] [2] [3]Core class refactoring:
Columnclass and related factory methods to operate onReadOnlyMemory<char>instead ofstring, updated the replacement pipeline to use memory-based functions, and added private helpers for tab, truncation, and null character replacement. [1] [2] [3] [4] [5] [6]ColumnizedLogLineto implement the new memory-based interface and expose both old and new properties for compatibility.Unit test adjustments:
ColumnTests.csto useAsMemory()for test values and to call.ToString()when checkingDisplayValuefor string assertions. Also added suppression attributes for globalization warnings in tests. [1] [2] [3] [4] [5]General codebase cleanup:
usingstatements from several interface files for clarity and maintainability. [1] [2] [3]These changes collectively modernize the columnizer infrastructure to be more memory-efficient and extensible, paving the way for future performance improvements and better support for large log files.