Skip to content

New reader - #506

Merged
Hirogen merged 42 commits into
Developmentfrom
new_reader
Dec 12, 2025
Merged

New reader#506
Hirogen merged 42 commits into
Developmentfrom
new_reader

Conversation

@Hirogen

@HirogenHirogen commented Dec 2, 2025

Copy link
Copy Markdown
Collaborator

This pull request introduces a new set of "Memory"-based interfaces and updates the Column and related classes to use ReadOnlyMemory<char> instead of string for 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:

  • Introduced new interfaces: IColumnMemory, IColumnizedLogLineMemory, IAutoLogLineMemoryColumnizerCallback, IColumnizerConfiguratorMemory, and IColumnizerPriorityMemory to support ReadOnlyMemory<char>-based operations and memory-efficient log processing. [1][2][3][4][5]
  • Updated existing interfaces and consumers to provide backward compatibility via [Obsolete] members and to reference new memory-based types where appropriate, such as in IColumnizedLogLine and IContextMenuEntry. [1][2][3]

Core class refactoring:

  • Refactored the Column class and related factory methods to operate on ReadOnlyMemory<char> instead of string, 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]
  • Updated ColumnizedLogLine to implement the new memory-based interface and expose both old and new properties for compatibility.

Unit test adjustments:

  • Updated unit tests in ColumnTests.cs to use AsMemory() for test values and to call .ToString() when checking DisplayValue for string assertions. Also added suppression attributes for globalization warnings in tests. [1][2][3][4][5]

General codebase cleanup:

  • Removed unused using statements from several interface files for clarity and maintainability. [1][2][3]
  • Added or improved XML documentation comments on interfaces for better code documentation. [1][2]

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.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ReaderType enum 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.md detailing 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
FileDescription
src/docs/performance/PIPELINES_IMPLEMENTATION_STRATEGY.mdAdds 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.csReduces minimum line length from 20,000 to 1,000 characters, providing more flexibility but potentially allowing problematic configurations
src/LogExpert.UI/Controls/LogWindow/LogWindow.csModernizes regex usage by converting to source-generated regexes using [GeneratedRegex] attribute for improved performance
src/LogExpert.Tests/IPC/ActiveWindowTrackingTests.csAdds comprehensive 312-line test suite for active window tracking functionality with scenario, edge case, and integration tests
src/LogExpert.Tests/ColumnizerPickerTest.csMinor formatting cleanup adding blank lines for improved readability in test LogLine implementation
src/LogExpert.Core/Interface/ILogStreamReaderSpan.csIntroduces new interface for span-based stream reading with memory management methods (marked internal, lacks documentation)
src/LogExpert.Core/Interface/ILogStreamReader.csCode formatting cleanup removing BOM and normalizing whitespace
src/LogExpert.Core/Classes/Log/PositionAwareStreamReaderPipeline.csAdds skeleton implementation of Pipeline-based reader with most methods throwing NotImplementedException - incomplete and non-functional
src/LogExpert.Core/Classes/Log/PositionAwareStreamReaderChannel.csImplements new Channel-based asynchronous log reader with pooled buffers and backpressure handling, now the default reader type
src/LogExpert.Core/Classes/Log/LogfileReader.csRefactors reader selection to use ReaderType enum with switch expression, removes LogLine record, hardcodes Channel reader as default
src/LogExpert.Core/Classes/Log/LogBuffer.csModernizes code with collection expressions, improved formatting, and refactored conditional logic
src/ColumnizerLib/ITextValue.csMarks interface as obsolete and adds extension methods for backward compatibility
src/ColumnizerLib/ILogLineSpanColumnizer.csIntroduces new interface for span-based columnizer operations to reduce string allocations
src/ColumnizerLib/ILogLineSpan.csDefines ILogLineSpan interface and LogLineSpan ref struct (has compilation errors - ref struct cannot implement interface)
src/ColumnizerLib/ILogLine.csAdds LogLine readonly struct implementation to replace previous record-based approach
src/ColumnizerLib/IInitColumnizer.csRemoves unused using statements and fixes trailing whitespace in documentation comments
src/ColumnizerLib/IFileSystemCallback.csRemoves unused using statements and normalizes method formatting
src/ColumnizerLib/IColumnizerPriority.csRemoves unused using statements and normalizes method formatting
src/ColumnizerLib/IColumnizerConfigurator.csFixes trailing whitespace in documentation comments and normalizes method formatting
src/ColumnizerLib/IColumnizedLogLine.csRemoves unused using statements and extra blank line
Files not reviewed (1)
  • src/LogExpert.UI/Dialogs/SettingsDialog.Designer.cs: Language not supported

Comment threadsrc/LogExpert.Core/Classes/Log/PositionAwareStreamReaderPipeline.cs Outdated
Comment threadsrc/LogExpert.Core/Classes/Log/PositionAwareStreamReaderPipeline.cs Outdated
Comment threadsrc/ColumnizerLib/ILogLineSpan.cs
Comment threadsrc/LogExpert.Core/Classes/Log/LogfileReader.cs Outdated
Comment threadsrc/LogExpert.Core/Classes/Log/PositionAwareStreamReaderChannel.cs Outdated
Comment threadsrc/LogExpert.UI/Controls/LogWindow/LogWindow.cs Outdated
Comment threadsrc/LogExpert.Core/Classes/Log/LogfileReader.cs Outdated
Comment threadsrc/LogExpert.Core/Classes/Log/PositionAwareStreamReaderChannel.cs Outdated
Comment threadsrc/LogExpert.Benchmarks/StreamReaderBenchmarks.cs Fixed
_updateIntervalMs = updateIntervalMs;

// Start timer
_timer = new Timer(ProcessQueue, null, updateIntervalMs, updateIntervalMs);
Comment threadsrc/LogExpert.Benchmarks/StreamReaderBenchmarks.cs Fixed
Comment threadsrc/LogExpert.Core/Classes/Log/BatchedProgressReporter.cs Fixed
Hirogenand others added 10 commits December 6, 2025 10:04
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>
Comment threadsrc/LogExpert.Core/Classes/Log/LogfileReader.cs Fixed

for (int i = 0; i < _steps.Count; i++)
{
_buffers[i] = new BlockingCollection<object>(100); // Bounded capacity
Comment threadsrc/LogExpert.Benchmarks/QuickPipelineTest.cs Fixed
Comment threadsrc/LogExpert.Core/Interface/IPipeline.cs Fixed
Comment threadsrc/LogExpert.Core/Classes/Columnizer/ColumnizerPicker.cs Fixed
Comment threadsrc/LogExpert.Core/Classes/Columnizer/ColumnizerPicker.cs Fixed
Comment threadsrc/LogExpert.Core/Callback/ColumnizerCallback.cs Fixed
ArgumentNullException.ThrowIfNull(logLine);
ArgumentNullException.ThrowIfNull(callback);

return new Log4JLogLine(ReplaceInMemory(logLine.FullLine, SEPARATOR_CHAR, '|'), logLine.Text, logLine.LineNumber);
Comment threadsrc/JsonColumnizer/JsonColumnizer.cs Fixed
ColumnList.Clear();
ColumnSet.Clear();

var line = callback.GetLogLineMemory(0);
Comment threadsrc/Log4jXmlColumnizer/Log4jXmlColumnizer.cs Fixed
/// <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;
@HirogenHirogen linked an issue Dec 12, 2025 that may be closed by this pull request
@Hirogen
Hirogen merged commit 88ff2f0 into DevelopmentDec 12, 2025
1 check passed
@Hirogen
Hirogen deleted the new_reader branch December 12, 2025 22:13
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refernced value types should not be part of an interface

2 participants

@Hirogen