Skip to content

Class tree performance: remove per-node N+1 queries (build + serialize)#302

Open
alexskr wants to merge 2 commits into
developfrom
fix/tree-scheme-n-plus-1
Open

Class tree performance: remove per-node N+1 queries (build + serialize)#302
alexskr wants to merge 2 commits into
developfrom
fix/tree-scheme-n-plus-1

Conversation

@alexskr

@alexskr alexskr commented Jul 3, 2026

Copy link
Copy Markdown
Member

Removes two independent per-node N+1 SPARQL patterns on the class tree endpoint. Follow-up to #297 (batch hasChildren resolution). Two separate commits, one per fix.

Query counts measured against staging AG (goo queries per /classes/:cls/tree).

Commit 1 — tree build: per-node concept-scheme query · Fixes #303

load_is_in_scheme runs per node during build and calls get_main_concept_schemeall_concepts_schemes (a SPARQL query). Two issues:

  • No skos? guardisInActiveScheme is SKOS-only and only serialized for SKOS, but non-SKOS (OWL/OBO) submissions still ran a skos:ConceptScheme query per node that always returns nothing. Now skipped for them.
  • No memoization — for genuine SKOS the scheme set is fixed per request but was re-queried per node. Now memoized on the submission.
Ontology Type Build queries before → after
IOBC OWL 669 → 92
AGROVOC SKOS 264 → 58

Files: concerns/concepts/concept_in_scheme.rb, concerns/ontology_submissions/skos/skos_submission_schemes.rb

Commit 2 — JSON serialize: per-node naturalLanguage query · Fixes #304

serializers/json.rb get_languages runs once per serialized object and calls submission.bring :naturalLanguage. goo's bring re-queries every call, so a response of N objects sharing a submission issued N queries. Guarded with bring? (the codebase convention) so it loads once per request.

This is not tree-specific — get_languages runs for every multi-class response (search, /children, /descendants, collections), so this helps broadly. It is also the dominant N+1 on large trees.

Ontology Type Nodes Serialize queries before → after
DDSS OWL 2945 2830 → 0
AGROVOC SKOS (multilingual) 173 60 → 0

File: serializers/json.rb

Testing

Syntax-checked; query counts verified against staging AG (numbers above). No triplestore-backed unit run yet — please run the class-tree controller tests against a backend before merge, ideally on both a SKOS and an OWL ontology.

Gate isInActiveScheme resolution by skos? and memoize all_concepts_schemes
on the submission. Non-SKOS trees now issue 0 scheme queries; SKOS trees
issue 1 instead of one per node.

#303
@alexskr alexskr force-pushed the fix/tree-scheme-n-plus-1 branch from f778c4b to 3f173e9 Compare July 3, 2026 05:26
@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 81.36%. Comparing base (a8a8dfb) to head (1c316d2).

Files with missing lines Patch % Lines
lib/ontologies_linked_data/serializers/json.rb 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #302   +/-   ##
========================================
  Coverage    81.36%   81.36%           
========================================
  Files          101      101           
  Lines         6848     6849    +1     
========================================
+ Hits          5572     5573    +1     
  Misses        1276     1276           
Flag Coverage Δ
ag 81.28% <75.00%> (+0.03%) ⬆️
fs 81.34% <75.00%> (+<0.01%) ⬆️
gd 81.28% <75.00%> (+0.04%) ⬆️
unittests 81.36% <75.00%> (+<0.01%) ⬆️
vo 81.29% <75.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.

@alexskr alexskr marked this pull request as draft July 3, 2026 06:02
@alexskr alexskr changed the title Fix N+1 SKOS scheme query in class tree endpoint Reduce class tree-build SPARQL: skip isInActiveScheme for non-SKOS, memoize concept schemes for SKOS Jul 3, 2026
get_languages runs once per serialized object; goo's #bring re-queries every
call, so a response of N objects sharing a submission issued N naturalLanguage
queries. Guard with bring? so it loads once per request. Affects all
multi-class responses (tree, search, children, descendants).

#304
@alexskr alexskr changed the title Reduce class tree-build SPARQL: skip isInActiveScheme for non-SKOS, memoize concept schemes for SKOS Class tree performance: remove per-node N+1 queries (build + serialize) Jul 3, 2026
@mdorf mdorf self-assigned this Jul 7, 2026

@mdorf mdorf left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the approach — it's sound and near-optimal for the goal. The fixes work because classes loaded .in(submission) share one submission instance, so the memoization and bring? guards collapse to one query per request. No correctness regression found.

Checked and cleared: the [nil][] change in load_is_in_scheme for non-SKOS can't affect the roots delete_if (ontology_submission.rb:814) — that filter only runs unless concept_schemes.empty?, while the altered branch only runs when schemes is empty. Mutually exclusive.

One observable change to document: for OWL/OBO ontologies, isInActiveScheme in the tree JSON goes from [null] to [] (it's in extra_include unconditionally and is serialized). That's a correctness improvement, but worth noting in the description and sanity-checking against the web UI.

Main gap — tests: needs a triplestore-backed class-tree test on both a SKOS and an OWL ontology, asserting correct output (ideally a query-count assertion so the N+1 can't silently return).

@mdorf mdorf marked this pull request as ready for review July 7, 2026 23:04
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