Class tree performance: remove per-node N+1 queries (build + serialize)#302
Class tree performance: remove per-node N+1 queries (build + serialize)#302alexskr wants to merge 2 commits into
Conversation
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
f778c4b to
3f173e9
Compare
Codecov Report❌ Patch coverage is
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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
There was a problem hiding this comment.
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).
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_schemeruns per node during build and callsget_main_concept_scheme→all_concepts_schemes(a SPARQL query). Two issues:skos?guard —isInActiveSchemeis SKOS-only and only serialized for SKOS, but non-SKOS (OWL/OBO) submissions still ran askos:ConceptSchemequery per node that always returns nothing. Now skipped for them.Files:
concerns/concepts/concept_in_scheme.rb,concerns/ontology_submissions/skos/skos_submission_schemes.rbCommit 2 — JSON serialize: per-node naturalLanguage query · Fixes #304
serializers/json.rbget_languagesruns once per serialized object and callssubmission.bring :naturalLanguage. goo'sbringre-queries every call, so a response of N objects sharing a submission issued N queries. Guarded withbring?(the codebase convention) so it loads once per request.This is not tree-specific —
get_languagesruns for every multi-class response (search,/children,/descendants, collections), so this helps broadly. It is also the dominant N+1 on large trees.File:
serializers/json.rbTesting
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.