Batch metadata query to avoid N+1 across 5 call sites#7
Open
Conversation
038ca7c to
738f4df
Compare
lookup_term_filtered called get_item() per scored ref — one SELECT and full deserialization per match. The filter only needs knowledge_type (a plain column) and range (json.loads of range_json), never the expensive knowledge_json deserialization (64% of per-row cost). Add get_metadata_multiple to ISemanticRefCollection that fetches only semref_id, range_json, knowledge_type in a single batch query. Replace the N+1 loop in lookup_term_filtered with one get_metadata_multiple call. Benchmark (200 matches, 200 rounds): 4.38ms → 1.32ms (3.3x speedup).
Apply the same get_metadata_multiple pattern from lookup_term_filtered to four more sites that called get_item() in a loop: - propindex.lookup_property_in_property_index: filter by .range - SemanticRefAccumulator.group_matches_by_type: group by .knowledge_type - SemanticRefAccumulator.get_matches_in_scope: filter by .range - answers.get_scored_semantic_refs_from_ordinals_iter: two-phase metadata filter then batch get_multiple for matching full objects All sites now use a single batch query instead of N individual SELECTs, skipping knowledge_json deserialization where only range or knowledge_type is needed.
parse_azure_endpoint returned the raw URL including ?api-version=... which AsyncAzureOpenAI then mangled into invalid paths like ...?api-version=2024-06-01/openai/. Strip the query string before returning — api_version is already returned as a separate value and passed to the SDK independently.
738f4df to
8332c44
Compare
…arisons - Use bisect_right with key=start in TextRangeCollection.contains_range to skip O(n) linear scan (O(log n) for non-overlapping point ranges) - Replace TextLocation allocations in TextRange __eq__/__lt__/__contains__ with a shared _effective_end returning tuples - Skip pydantic validation in get_metadata_multiple by constructing TextLocation/TextRange directly from JSON
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
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.
Summary
get_item()per scored ref — one SELECT and full deserialization per match (N+1 pattern)get_metadata_multipletoISemanticRefCollectionthat fetches onlysemref_id, range_json, knowledge_typein a single batch queryget_metadata_multiplecall at each sitecontains_range, inline tuple comparisons inTextRange, skip pydantic validation inget_metadata_multipleCall sites optimized
lookup_term_filtered— batch metadata, filter by knowledge_type/rangelookup_property_in_property_index— batch metadata, filter by range scopeSemanticRefAccumulator.group_matches_by_type— batch metadata, group by knowledge_typeSemanticRefAccumulator.get_matches_in_scope— batch metadata, filter by range scopeget_scored_semantic_refs_from_ordinals_iter— two-phase: metadata filter then batch fetchAdditional optimizations
TextRangeCollection.contains_range: replaced O(n) linear scan withbisect_rightkeyed onstart, reducing scope-filtering from ~24ms to ~9msTextRange: replacedTextLocationallocations in__eq__/__lt__/__contains__with a shared_effective_endreturning tuplesget_metadata_multiple: constructTextLocation/TextRangedirectly from JSON instead of going through__pydantic_validator__Bugfix included
parse_azure_endpointwas returning the full URL with query string (?api-version=...), whichAsyncAzureOpenAImangled into a double-path. Now strips the query string before returning. Added 6 unit tests.Benchmarks (pytest-async-benchmark pedantic, 200 rounds, Standard_D2s_v5)
200 matches against a 200-message indexed SQLite transcript. Only the function under test is timed.
lookup_term_filteredgroup_matches_by_typeget_scored_semantic_refs_from_ordinals_iterlookup_property_in_property_indexget_matches_in_scopeThe scope-filtering benchmarks (#4, #5) improved from 1.08x/1.06x to 2.61x/2.62x after adding binary search in
contains_rangeand inline tuple comparisons. The scored-refs benchmark trades Nget_itemcalls for metadata +get_multiple, roughly breaking even.How to run
pip install 'pytest-async-benchmark @ git+https://github.com/KRRT7/pytest-async-benchmark.git@feat/pedantic-mode' pytest-asyncio uv run python -m pytest tests/benchmarks/test_benchmark_query.py -v -sTest plan