perf: Optimize PMO StreamHub for O(1) incremental updates (P012)#1979
perf: Optimize PMO StreamHub for O(1) incremental updates (P012)#1979DaveSkender merged 6 commits intov3from
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-authored-by: DaveSkender <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/m-r/Pmo/Pmo.StreamHub.cs`:
- Around line 156-160: RollbackState currently clears state then bails when
ProviderCache.IndexGte(timestamp) returns <= 0, which lets histories stay
cleared if IndexGte returns -1; change RollbackState to handle the -1 case and
replay the cache instead of leaving state empty: after computing targetIndex =
ProviderCache.IndexGte(timestamp) treat -1 as "replay everything" (e.g. set
targetIndex = ProviderCache.Count), then clear state and replay cached items
from index 0 up to targetIndex - 1 (exclusive) to rebuild histories; reference
the RollbackState method, ProviderCache.IndexGte, targetIndex and the replay
loop when implementing this fix.
DaveSkender
temporarily deployed
to
stockindicators.dev
GitHub Actions
Inactive
Signed-off-by: Dave Skender <[email protected]>
DaveSkender
temporarily deployed
to
stockindicators.dev
GitHub Actions
Inactive
Signed-off-by: Dave Skender <[email protected]>
DaveSkender
temporarily deployed
to
stockindicators.dev
GitHub Actions
Inactive
|
@copilot In the base branch (now merged into this branch) we made a fairly significant set of changes to how rollbacks occur (see PR #1978). Consider if this impacts how you implement the fix for this solution. In any case, resolve the CI check failures and ensure a clean code completion with no lint, build, or test failures. |
Co-authored-by: DaveSkender <[email protected]>
Adapted to the new |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/m-r/Pmo/Pmo.StreamHub.cs (1)
149-196: RollbackState correctly adapted to newint restoreIndexAPI.The implementation properly:
- Clears state before replay (lines 152-154)
- Handles negative index sentinel by early return (line 156-159)
- Replays items 0 through
restoreIndexinclusive (line 164) to rebuild EMA stateThe past review comment about
IndexGte == -1handling is no longer applicable since the API changed fromRollbackState(DateTime timestamp)toRollbackState(int restoreIndex)per PR#1978— the framework now passes the index directly.Consider extracting the ROC/ROC EMA calculation (lines 169-194) into a private helper method to eliminate duplication with
ToIndicator(lines 68-94). This would improve maintainability if the calculation logic ever needs adjustment.♻️ Optional refactor to reduce duplication
+ private void ComputeRocState(int index) + { + double currVal = ProviderCache[index].Value; + double prevVal = index > 0 ? ProviderCache[index - 1].Value : double.NaN; + + double roc = prevVal == 0 ? double.NaN : 100 * ((currVal / prevVal) - 1); + _rocHistory.Add(roc); + + double rocEma; + if (double.IsNaN(_prevRocEma) && index >= TimePeriods) + { + double sum = 0; + for (int p = index - TimePeriods + 1; p <= index; p++) + { + sum += _rocHistory[p]; + } + rocEma = sum / TimePeriods; + } + else if (!double.IsNaN(_prevRocEma)) + { + rocEma = _prevRocEma + (smoothingConstant2 * (roc - _prevRocEma)); + } + else + { + rocEma = double.NaN; + } + + _prevRocEma = rocEma; + _rocEmaHistory.Add(rocEma * 10); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/m-r/Pmo/Pmo.StreamHub.cs` around lines 149 - 196, RollbackState correctly replays EMA state but duplicates the ROC/ROC-EMA logic used in ToIndicator; extract the shared computation into a private helper (e.g., ComputeRocAndEma or UpdateRocEma) that accepts the current index and the ProviderCache/values (or just currVal/prevVal) and returns/updates roc and rocEma (using TimePeriods, smoothingConstant2, _prevRocEma, _rocHistory, _rocEmaHistory as needed); replace the duplicated loop body in RollbackState and the corresponding block in ToIndicator with calls to this helper so both use the same logic and avoid maintenance drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/m-r/Pmo/Pmo.StreamHub.cs`:
- Around line 149-196: RollbackState correctly replays EMA state but duplicates
the ROC/ROC-EMA logic used in ToIndicator; extract the shared computation into a
private helper (e.g., ComputeRocAndEma or UpdateRocEma) that accepts the current
index and the ProviderCache/values (or just currVal/prevVal) and returns/updates
roc and rocEma (using TimePeriods, smoothingConstant2, _prevRocEma, _rocHistory,
_rocEmaHistory as needed); replace the duplicated loop body in RollbackState and
the corresponding block in ToIndicator with calls to this helper so both use the
same logic and avoid maintenance drift.
RollbackState(DateTime)withRollbackState(int restoreIndex)PruneState(DateTime)override for cache pruning alignment💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.