Skip to content

optimize matrix enumeration#45

Merged
liuhaoyang merged 5 commits intodotnetcore:mainfrom
eventhorizon-cli:optimize_matrix_enumeration
Jan 31, 2026
Merged

optimize matrix enumeration#45
liuhaoyang merged 5 commits intodotnetcore:mainfrom
eventhorizon-cli:optimize_matrix_enumeration

Conversation

@eventhorizon-cli
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Jan 15, 2026

Codecov Report

❌ Patch coverage is 90.90909% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.37%. Comparing base (8bdc420) to head (6a159be).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
.../Mocha.Query/Prometheus/PromQL/Engine/Evaluator.cs 72.72% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main      #45       +/-   ##
===========================================
- Coverage   91.64%   71.37%   -20.27%     
===========================================
  Files          51      156      +105     
  Lines         993     4010     +3017     
  Branches      107      538      +431     
===========================================
+ Hits          910     2862     +1952     
- Misses         57      990      +933     
- Partials       26      158      +132     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@liuhaoyang
Copy link
Member

Code Review

✅ 优点

  1. 性能优化思路正确 - 从每次全量 LINQ 查询改为使用可复用的枚举器,利用滑动窗口减少重复扫描
  2. 测试覆盖充分 - MatrixEnumeratorTests 包含了边缘情况的测试
  3. 基准测试完整 - 提供了性能基准测试验证优化效果
  4. 代码结构清晰 - MatrixEnumerator 类职责单一

⚠️ 建议改进

1. 参数验证缺失

MatrixEnumerator.Enumerate 方法缺少参数验证,建议添加:

if (reusedPoints == null)
    throw new ArgumentNullException(nameof(reusedPoints));

if (minTs > maxTs)
    throw new ArgumentException("minTs must be less than or equal to maxTs");

2. 样本有序性假设

代码假设 TimeSeriesSample 列表是按时间戳有序的。建议:

  • 在 XML 注释中明确说明这个前提
  • 或添加 Debug.Assert 确保输入有序

3. 覆盖率问题

Codecov 报告显示 Evaluator.cs 有 1 个缺失行和 2 个部分行需要测试覆盖。

4. 考虑 ArrayPool

虽然代码中还有 TODO: Use ArrayPool,但作为后续优化,使用 ArrayPool<T> 可以进一步减少内存分配。


📝 建议

  1. 添加参数验证的单元测试
  2. 在文档中明确说明样本必须有序的假设
  3. 补充缺失的测试覆盖率

总体评价:LGTM with minor improvements needed 👍

@liuhaoyang
Copy link
Member

Thanks for contributing this optimization! 🚀

Your implementation of the sliding MatrixEnumerator is a great approach to reduce unnecessary full scans and improve performance. The comprehensive test coverage and benchmark setup are also very helpful for validating the optimization.

Looking forward to seeing the performance improvements this brings to the matrix enumeration logic!

@liuhaoyang liuhaoyang merged commit aab54f5 into dotnetcore:main Jan 31, 2026
2 of 3 checks passed
Sign up for free to 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.

2 participants