fix: improve chart performance with large datasets using LTTB downsampling#457
Closed
fix: improve chart performance with large datasets using LTTB downsampling#457
Conversation
…pling (#36) When displaying 100k+ samples, OxyPlot rendered every data point, making the UI unusable. Implements the Largest-Triangle-Three-Buckets (LTTB) algorithm to downsample data to 5k points per channel while preserving visual shape (peaks, troughs, spikes). - Add DataPointDecimator with LTTB and gap-aware decimation - PlotLogger: increase raw buffer to 50k, render decimated 5k points - DatabaseLogger: replace 1M hard cap with per-channel LTTB decimation - DatabaseLogger: add OrderBy to ensure time-sorted data for LTTB - Add 15 unit tests covering correctness, fidelity, and performance Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Review Summary by QodoImplement LTTB downsampling for large dataset chart performance
WalkthroughsDescription• Implements LTTB downsampling algorithm to handle large datasets efficiently - Reduces rendered points to ~5,000 per channel while preserving visual shape - Preserves peaks, troughs, and spikes in time-series data • Increases PlotLogger raw buffer from 5k to 50k points per channel - Decimates to 5k for rendering every 1s cycle - Uses gap-aware decimation to preserve line breaks at data stream gaps • Replaces DatabaseLogger 1M point hard cap with per-channel LTTB decimation - Displays all data instead of silently dropping excess points - Adds OrderBy(TimestampTicks) to ensure time-sorted data for correct decimation • Adds 15 comprehensive unit tests covering edge cases, visual fidelity, and performance Diagramflowchart LR
A["Raw Data Points<br/>50k per channel"] -->|"Decimate every 1s"| B["DataPointDecimator<br/>LTTB Algorithm"]
B -->|"Gap-aware"| C["5k Display Points<br/>with gap markers"]
C --> D["PlotLogger<br/>Live Chart"]
E["Database Samples<br/>All records"] -->|"OrderBy Timestamp"| F["DataPointDecimator<br/>Per-channel LTTB"]
F --> G["Decimated Points<br/>Visual fidelity preserved"]
G --> H["DatabaseLogger<br/>Session Playback"]
File Changes1. Daqifi.Desktop/Loggers/DataPointDecimator.cs
|
Contributor
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+302
to
+312
| // Decimate raw points into the display lists bound to each series | ||
| // Use DecimateWithGaps to preserve line breaks at data stream gaps | ||
| foreach (var kvp in LoggedPoints) | ||
| { | ||
| if (_decimatedPoints.TryGetValue(kvp.Key, out var displayPoints)) | ||
| { | ||
| var decimated = DataPointDecimator.DecimateWithGaps(kvp.Value); | ||
| displayPoints.Clear(); | ||
| displayPoints.AddRange(decimated); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
3. Cross-thread series mutation 🐞 Bug ☼ Reliability
PlotLogger can mutate PlotModel.Series and its point dictionaries from device sample callbacks that originate in async/background message handling, but AddChannelSeries performs those mutations without holding PlotModel.SyncRoot. This can lead to races with UI rendering (CompositionTarget.Rendering) and crashes or corrupted plot state.
Agent Prompt
### Issue description
`AddChannelSeries` mutates plot state (PlotModel.Series + dictionaries) without holding the same synchronization used elsewhere (`PlotModel.SyncRoot`). Streaming updates originate from async/background message handling, so this can race with UI rendering.
### Issue Context
`CompositionTarget.Rendering` is on the UI thread and enumerates `LoggedPoints` under `lock (PlotModel.SyncRoot)`. `PlotLogger.Log` only locks around point appends/removals, not around series/dictionary creation.
### Fix Focus Areas
- Wrap the `ContainsKey`/`AddChannelSeries` path inside `lock (PlotModel.SyncRoot)` (or otherwise ensure it runs on the UI thread via Dispatcher).
- Ensure all mutations of `LoggedPoints`, `_decimatedPoints`, `LoggedChannels`, and `PlotModel.Series` are guarded consistently.
- Consider also guarding `LoggedChannels[key].Color` updates similarly.
- Daqifi.Desktop/Loggers/PlotLogger.cs[186-224]
- Daqifi.Desktop/Loggers/PlotLogger.cs[238-276]
- Daqifi.Desktop/Loggers/PlotLogger.cs[280-316]
- Daqifi.Desktop/Device/AbstractStreamingDevice.cs[219-234]
- Daqifi.Desktop/Device/AbstractStreamingDevice.cs[236-310]
- Daqifi.Desktop/Channel/AbstractChannel.cs[136-161]
- Daqifi.Desktop/Loggers/LoggingManager.cs[423-443]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
256
to
265
| var dbSamples = context.Samples.AsNoTracking() | ||
| .Where(s => s.LoggingSessionID == session.ID) | ||
| .OrderBy(s => s.TimestampTicks) | ||
| .Select(s => new { s.ChannelName, s.DeviceSerialNo, s.Type, s.Color, s.TimestampTicks, s.Value }) | ||
| .ToList(); // Bring data into memory | ||
| .ToList(); | ||
|
|
||
| var samplesCount = dbSamples.Count; | ||
| const int dataPointsToShow = 1000000; | ||
|
|
||
| if (samplesCount > dataPointsToShow) | ||
| { | ||
| subtitle = $"\nOnly showing {dataPointsToShow:n0} out of {samplesCount:n0} data points"; | ||
| } | ||
|
|
||
| var channelInfoList = dbSamples | ||
| .Select(s => new { s.ChannelName, s.DeviceSerialNo, s.Type, s.Color }) |
Contributor
There was a problem hiding this comment.
4. Playback builds full point lists 🐞 Bug ☼ Reliability
DatabaseLogger.DisplayLoggingSession materializes all samples into memory and then builds full per-channel DataPoint lists before decimating, which can spike peak memory and slow loads for very large sessions. The previous loop stopped adding after 1M points; now every sample is added to per-channel lists before downsampling.
Agent Prompt
### Issue description
Session playback now appends *all* samples into `_allSessionPoints` before decimating, increasing peak memory and processing time for very large sessions.
### Issue Context
Even though the final `ItemsSource` is decimated, the peak load includes `dbSamples` + full `_allSessionPoints` lists.
### Fix Focus Areas
- Avoid storing `dbSamples` and full per-channel lists at the same time.
- Prefer querying/processing per channel (e.g., query distinct channels first, then for each channel run an ordered query and decimate) to reduce peak memory.
- Consider streaming enumeration (no `ToList`) and/or reducing intermediate allocations.
- Daqifi.Desktop/Loggers/DatabaseLogger.cs[256-304]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
- DecimateWithGaps: use floor allocation with remainder distribution to enforce the global threshold cap, preventing output from exceeding the requested point budget when many gaps exist - PlotLogger: replace List<DataPoint> with CircularBuffer<DataPoint> for the raw point storage, eliminating O(n) RemoveAt(0) on every sample at the 50k buffer size - Add CircularBuffer<T> with O(1) Add, indexed access, and ToList() - Add tests for CircularBuffer and DecimateWithGaps threshold enforcement Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📊 Code Coverage ReportSummarySummary
CoverageDAQiFi - 20.2%
Daqifi.Desktop.Common - 33.3%
Daqifi.Desktop.IO - 100%
Coverage report generated by ReportGenerator • View full report in build artifacts |
7 tasks
Contributor
Author
|
will come up with a better solution |
7 tasks
tylerkron
added a commit
that referenced
this pull request
Apr 11, 2026
ADR 001 documents why we chose viewport-aware MinMax downsampling over global LTTB (PR #457), pre-computed pyramids, GPU rendering, and on-demand DB queries. Records the trade-offs, consequences, and follow-up work for future contributors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
tylerkron
added a commit
that referenced
this pull request
Apr 12, 2026
ADR 001 documents why we chose viewport-aware MinMax downsampling over global LTTB (PR #457), pre-computed pyramids, GPU rendering, and on-demand DB queries. Records the trade-offs, consequences, and follow-up work for future contributors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.
Summary
Closes #36
OrderBy(TimestampTicks)to ensure time-sorted data for correct decimationDataPointDecimatorwith both standard and gap-aware decimation methodsTest plan
🤖 Generated with Claude Code