Skip to content

Optimize UI5BindingPath.getNode() and constructPathStringInner evaluation performance#350

Closed
Copilot wants to merge 5 commits intomainfrom
copilot/optimize-ui5bindingpath-performance
Closed

Optimize UI5BindingPath.getNode() and constructPathStringInner evaluation performance#350
Copilot wants to merge 5 commits intomainfrom
copilot/optimize-ui5bindingpath-performance

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 31, 2026

What This PR Contributes

Profiling revealed UI5BindingPath.getNode() accounts for 98.4% of evaluation time (~33 minutes) when UI5Xss.ql runs standalone on large UI5 monorepo databases — a ~240x slowdown for ~2x data size. Root cause: disjuncts 1-1 and 1-2 lacked pragma[nomagic], causing the evaluator to inline their joins into a single relation and producing a cross-product explosion.

  • Critical — getNode() in UI5View.qll: Extract disjuncts 1-1 and 1-2 into dedicated pragma[nomagic] helper predicates (getHardcodedJsonModelNode, getJsonFileModelNode), matching the pattern already used for disjuncts 1-3 and 2
    Node getNode() {
      result = getHardcodedJsonModelNode(this)   // was inline exists(Property p, JsonModel model | ...)
      or
      result = getJsonFileModelNode(this)        // was inline exists(string propName, JsonModel model | ...)
      or
      result = getNonStaticJsonModelNode(this)   // already pragma[nomagic] ✓
      or
      result = getExternalModelNode(this)        // already pragma[nomagic] ✓
    }
  • Moderate — constructPathStringInner in UI5.qll: Add pragma[nomagic] to both recursive overloads to prevent inlining into callers

All 74 existing UI5 tests pass unchanged.

Future Works

  • CFG traversal in CdsTransaction.getContextObject(): Investigated restricting getAPredecessor+() to same container, but cds.context assignments can legitimately be in outer scopes. Needs a more nuanced scoping strategy.
  • TVarRefinementContext baseline cost: This is a codeql/javascript-all predicate — not addressable here. Custom queries could potentially be restructured to reduce dependence on full type inference.
  • Performance regression test: Establishing a standalone UI5Xss.ql eval-time gate on a large UI5 monorepo database.

Copilot AI and others added 4 commits March 31, 2026 02:13
Agent-Logs-Url: https://github.com/advanced-security/codeql-sap-js/sessions/b8d4d32d-c84b-4174-bc1e-f00aa8b7ede4

Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
…mance

- Extract getNode() disjuncts 1-1 and 1-2 into pragma[nomagic] helper
  predicates (getHardcodedJsonModelNode, getJsonFileModelNode) to prevent
  cross-product explosion on large codebases. This matches the existing
  pattern used for disjuncts 1-3 and 2.
- Add pragma[nomagic] to both constructPathStringInner recursive predicates
  to prevent inlining into calling contexts.

Addresses the critical UI5Xss.ql ~240x performance regression on large
databases where getNode() accounted for 98.4% of evaluation time.

Agent-Logs-Url: https://github.com/advanced-security/codeql-sap-js/sessions/b8d4d32d-c84b-4174-bc1e-f00aa8b7ede4

Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
Copilot AI changed the title [WIP] Optimize query evaluation performance for UI5BindingPath.getNode() Optimize UI5BindingPath.getNode() and constructPathStringInner evaluation performance Mar 31, 2026
Copilot AI requested a review from data-douser March 31, 2026 02:40
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.

Query evaluation performance: UI5BindingPath.getNode() bottleneck and other optimization opportunities

2 participants