From beb8527defc766543fe0e22de0250a876e35fc42 Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov Date: Fri, 10 Dec 2021 17:32:30 +0300 Subject: [PATCH] Don't excessively look up the extension oid Use the one from the static variable. This improves performance by avoiding excessive catalog lookups. --- src/extension.c | 25 ++++++++++++++++++++++++- src/extension.h | 2 ++ src/planner.c | 9 +++++++++ tsl/src/fdw/relinfo.c | 24 +++++++++++++----------- 4 files changed, 48 insertions(+), 12 deletions(-) diff --git a/src/extension.c b/src/extension.c index 3dbd320e0..32c07a0d3 100644 --- a/src/extension.c +++ b/src/extension.c @@ -57,6 +57,13 @@ static Oid extension_proxy_oid = InvalidOid; static enum ExtensionState extstate = EXTENSION_STATE_UNKNOWN; +/* + * Looking up the extension oid is a catalog lookup that can be costly, and we + * often need it during the planning, so we cache it here. We update it when + * the extension status is updated. + */ +Oid ts_extension_oid = InvalidOid; + static bool extension_loader_present() { @@ -158,7 +165,23 @@ extension_update_state() return; in_recursion = true; - extension_set_state(extension_current_state()); + enum ExtensionState new_state = extension_current_state(); + extension_set_state(new_state); + /* + * Update the extension oid. Note that it is only safe to run + * get_extension_oid() when the extension state is 'CREATED', because + * otherwise we might not be even able to do a catalog lookup because we + * are not in transaction state, and the like. + */ + if (new_state == EXTENSION_STATE_CREATED) + { + ts_extension_oid = get_extension_oid(EXTENSION_NAME, true /* missing_ok */); + Assert(ts_extension_oid != InvalidOid); + } + else + { + ts_extension_oid = InvalidOid; + } in_recursion = false; } diff --git a/src/extension.h b/src/extension.h index 89ca84e0f..3966589b2 100644 --- a/src/extension.h +++ b/src/extension.h @@ -19,4 +19,6 @@ extern const char *ts_experimental_schema_name(void); extern const char *ts_extension_get_so_name(void); extern TSDLLEXPORT const char *ts_extension_get_version(void); +extern TSDLLEXPORT Oid ts_extension_oid; + #endif /* TIMESCALEDB_EXTENSION_H */ diff --git a/src/planner.c b/src/planner.c index be8c7eb20..02e4ebd68 100644 --- a/src/planner.c +++ b/src/planner.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -348,6 +349,14 @@ timescaledb_planner(Query *parse, int cursor_opts, ParamListInfo bound_params) context.rootquery = parse; if (ts_extension_is_loaded()) { + /* + * Some debug checks. + */ + Assert(ts_extension_oid == get_extension_oid(EXTENSION_NAME, /* missing_ok = */ false)); + + /* + * Preprocess the hypertables in the query and warm up the caches. + */ preprocess_query((Node *) parse, &context); /* diff --git a/tsl/src/fdw/relinfo.c b/tsl/src/fdw/relinfo.c index 46bff52a1..b7c05a8b8 100644 --- a/tsl/src/fdw/relinfo.c +++ b/tsl/src/fdw/relinfo.c @@ -18,19 +18,20 @@ #include #include -#include "remote/connection.h" -#include "option.h" -#include "deparse.h" -#include "relinfo.h" -#include "estimate.h" -#include "chunk_adaptive.h" #include "cache.h" +#include "chunk.h" +#include "chunk_adaptive.h" +#include "deparse.h" +#include "dimension.h" +#include "errors.h" +#include "estimate.h" +#include "extension.h" +#include "hypercube.h" #include "hypertable.h" #include "hypertable_cache.h" -#include "dimension.h" -#include "chunk.h" -#include "hypercube.h" -#include "errors.h" +#include "option.h" +#include "relinfo.h" +#include "remote/connection.h" #include "scan_exec.h" /* Default CPU cost to start up a foreign query. */ @@ -427,7 +428,8 @@ fdw_relinfo_create(PlannerInfo *root, RelOptInfo *rel, Oid server_oid, Oid local */ fpinfo->fdw_startup_cost = DEFAULT_FDW_STARTUP_COST; fpinfo->fdw_tuple_cost = DEFAULT_FDW_TUPLE_COST; - fpinfo->shippable_extensions = list_make1_oid(get_extension_oid(EXTENSION_NAME, true)); + Assert(ts_extension_oid != InvalidOid); + fpinfo->shippable_extensions = list_make1_oid(ts_extension_oid); fpinfo->fetch_size = DEFAULT_FDW_FETCH_SIZE; apply_fdw_and_server_options(fpinfo);