⚡ Optimize config version pruning to avoid N+1 queries #65
Conversation
Replaced the N+1 deletion loop (fetching all versions, slicing in memory, and deleting individually) with a single efficient DELETE query using a cutoff strategy. This change: - Reduces memory usage by only fetching the Nth newest record (cutoff). - Reduces database roundtrips from O(N) to O(1) (1 select + 1 delete). - Prevents race conditions by using a deterministic cutoff (created_at + id) rather than a `whereNotIn` exclusion list. - Includes a regression/benchmark test verifying correctness and performance (~6ms execution).
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
QA Pipeline Results
Artifacts
Generated by core php qa pipeline |
- Removed `2>&1` stderr redirection from Psalm, PHPStan, Pint, and Composer Audit commands. This prevents progress messages and warnings (which are written to stderr) from corrupting the JSON/SARIF output files (which are captured via stdout redirection). - Upgraded `github/codeql-action/upload-sarif` from v3 to v4 to resolve deprecation warnings and improve handling of SARIF files (including empty location fixes). - This resolves the "Invalid SARIF. JSON syntax error" failure in the QA pipeline.
Psalm occasionally generates SARIF reports with location coordinates of 0 (e.g., `startLine: 0`), which violates the SARIF spec (minimum 1) and causes the `github/codeql-action/upload-sarif` action to fail. This change adds a `jq` post-processing step to the QA workflow that recursively sanitizes the SARIF file, ensuring all region coordinates (lines and columns) are at least 1.
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
The previous sanitization logic assumed a specific parent structure (`region` object). This updated logic recursively traverses the entire JSON structure and targets any key matching `startLine`, `startColumn`, `endLine`, or `endColumn`, forcing their values to be at least 1. This ensures full compliance with the SARIF schema regardless of where these coordinates appear.
Performance Optimization: Config Version Pruning
💡 What
Optimized
ConfigVersioning::pruneOldVersionsto replace the inefficient N+1 deletion pattern with a direct database query.🎯 Why
The previous implementation loaded all configuration versions into memory and then iterated through them to delete old ones individually. This caused:
🔧 Implementation Details
DELETEstatement for all records strictly older than the cutoff version (usingcreated_atandidfor deterministic comparison).whereNotInapproach, the cutoff strategy is safe against concurrent insertions of new versions, as they will be newer than the cutoff and thus preserved.📊 Measured Improvement
Benchmark test (
ConfigVersioningPerformanceTest) shows:maxVersions(50) are kept.PR created automatically by Jules for task 6708350155386618536 started by @Snider