fix(gap-analysis): repair dead GAP_ANALYSIS_OPTIMIZED toggle + add benchmark harness#748
Open
PRAteek-singHWY wants to merge 1 commit intoOWASP:mainfrom
Open
Conversation
The toggle added in PR OWASP#717 was being overridden by a duplicate gap_analysis method left over from PR OWASP#716. Removed the duplicate so the feature toggle actually works as intended. Also adds scripts/benchmark_gap.py which proved the optimized mode is 99.5% faster and uses 99.6% less memory than the original. Closes OWASP#587
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.
What & Why
Bug: Feature toggle introduced in PR #717 was silently broken
When PR #717 was stacked on PR #716 and both were merged, a duplicate
gap_analysismethod was accidentally left inNEO_DB(the PR #716 version at line ~733). In Python, when two methods share the same name in a class, the last one wins — so the toggle wrapper from PR #717 (line ~563) was completely shadowed.Effect:
GAP_ANALYSIS_OPTIMIZEDhad zero effect. Everyone was hitting the tiered-pruning path unconditionally — with no way to fall back to the safe exhaustive traversal @robvanderveer requested as a feature toggle.Fix
Removed the duplicate
gap_analysismethod. The toggle now works correctly:GAP_ANALYSIS_OPTIMIZEDfalse/ unset (default)_gap_analysis_original— original exhaustive traversal, safe defaulttrue_gap_analysis_optimized— Tier 1 → 2 → 3 tiered pruning with early exitBenchmark — closes #587
Added
scripts/benchmark_gap.py— the empirical proof requested in Issue #587 that a previous contributor never delivered. Run it yourself against any live Neo4j instance:Results — local Neo4j (19 standards loaded, 3 runs each)
GAP_ANALYSIS_OPTIMIZED=false)GAP_ANALYSIS_OPTIMIZED=true)33.07s0.16s37.32 MB0.17 MB6289322(always both)1(early exit at Tier 1)Files Changed
application/database/db.py— removed duplicategap_analysismethod (67 lines deleted)scripts/benchmark_gap.py— new benchmark script (258 lines added); runs both modes head-to-head and prints a GitHub-ready results tablefixes #587