perf(catalog): reuse loaded catalog state in pending_sync_rows - #22
Open
aditya3799 wants to merge 1 commit into
Open
perf(catalog): reuse loaded catalog state in pending_sync_rows#22aditya3799 wants to merge 1 commit into
aditya3799 wants to merge 1 commit into
Conversation
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.
What does this PR do?
Building on the excellent optimization in #19, this PR removes the remaining redundant catalog reads from the
ensure_current_graph()hot path.Currently, even after #19 removes the duplicate lookup in
current_catalog_state(), the very next line callspending_sync_rows(). This triggers a fullread_catalog()viaSyncReplayContext::load(), which meansensure_current_graph()is still reading the entire catalog a second time (including executing two massive SPI queries for_registered_tablesand_registered_edges).How it works
This PR extracts the
applicable_table_oidsfrom thecatalog_statethat we already loaded at the top ofensure_current_graph(), and passes them directly down intopending_sync_rows.By bypassing
SyncReplayContext::load(), we completely eliminate all redundant SPI catalog queries from the traversal setup cost. This should completely eliminate the remainder of the ~3.5ms unamortized floor identified in the parent issue.Checklist
catalog_stateinruntime.rssql_sync.rsdirect functions_pending_sync_rows_for_current_roleABI inadmin.rsCloses #21