Tal.ba/opt mem allocation - #1660
TalBarYakar wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a8fd7cf. Configure here.
| needs: [prepare-values] | ||
| uses: ./.github/workflows/flow-memory-comparison.yml | ||
| with: | ||
| redis-ref: ${{ needs.prepare-values.outputs.redis-ref }} |
There was a problem hiding this comment.
Comparison job lacks PR skip guards
Low Severity
memory-comparison only needs prepare-values and has no if condition. Unlike memory-regression, it still runs on draft PRs and docs-only changes. Each run builds Redis plus two release modules, downloads the city fixture, and can take up to 45 minutes, even though the job cannot fail the PR.
Reviewed by Cursor Bugbot for commit a8fd7cf. Configure here.
9f47c60 to
07158c1
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1660 +/- ##
=======================================
Coverage 88.06% 88.06%
=======================================
Files 17 17
Lines 5957 5957
=======================================
Hits 5246 5246
Misses 711 711 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|


Note
Medium Risk
Touches the core JSON parse path and a git-pinned ijson dependency; behavior is guarded by depth/readback tests and informational CI, but allocation changes could surface subtle regressions.
Overview
Switches JSON string ingestion to compact object allocation by bumping the pinned ijson revision and calling
deserialize_compact_objectsinstead of the standard deserializer infrom_str. Parsed JSON should remain equivalent; object backing storage and reportedJSON.DEBUG MEMORYsizes can change.Adds a non-gating
memory-comparisonGitHub Actions job (PR CI and nightly) that builds current vs masterlibrejson.so, runstests/pytest/memory_comparison.py, and publishes an artifact/summary for process memory, per-key memory, and replacementJSON.SETtiming. Pytest updates replace fixedJSON.DEBUG MEMORYbyte expectations with checks that default-path memory is positive and matches the explicit$path.A new unit test asserts compact parsing still honors recursion/depth limits and preserves serialized output.
Reviewed by Cursor Bugbot for commit 07158c1. Bugbot is set up for automated code reviews on this repo. Configure here.