Profiler driven optimization for perspective-server

Signed-off-by: Andrew Stein <steinlink@gmail.com>

Author: texodus

pnpm-workspace.yaml

@@ -42,7 +42,6 @@ catalog:
     # Dev Dependencies
     "@clickhouse/client-web": "^1.12.0"
     "@duckdb/duckdb-wasm": "1.33.1-dev18.0"
-    "@duckdb/duckdb-wasm-shell": "^1.30.0"
     "@fontsource/roboto-mono": "4.5.10"
     "@iarna/toml": "3.0.0"
     "@jupyterlab/builder": "^4"

rust/perspective-server/cpp/perspective/CMakeLists.txt

@@ -576,23 +576,12 @@ else()
 # -s MEMORY_GROWTH_GEOMETRIC_STEP=1.0 \
 # -s MEMORY_GROWTH_GEOMETRIC_CAP=536870912 \
 
+    set(PSP_FILESYSTEM_FLAGS "-s NO_FILESYSTEM=1 ")
+    set(PSP_CLOSURE_FLAGS "--closure=1 ")
+
     # On-disk tables in the browser are backed by OPFS via emscripten's WasmFS.
-    # `PSP_WASM_OPFS` swaps the default `NO_FILESYSTEM` (memory-only) build for
-    # `WASMFS` + `FORCE_FILESYSTEM`, and enables the OPFS backend. `JSPI` provides
-    # the synchronous-looking OPFS access the WasmFS OPFS backend needs without
-    # SharedArrayBuffer/cross-origin-isolation (`-pthread`). `JSPI` is backwards
-    # compatible: the module loads and runs in-memory tables fine on non-JSPI
-    # browsers (it has no `main`/async imports wrapped at instantiation); the JSPI
-    # API is only reached when an actual OPFS op runs, i.e. when `page_to_disk` is used
-    # (the JS client gates that on JSPI being present). Closure is incompatible
-    # with the WasmFS JS glue, so it is disabled in that case.
     if(PSP_WASM_OPFS)
-        set(PSP_FILESYSTEM_FLAGS "-s NO_FILESYSTEM=1 ")
-        set(PSP_CLOSURE_FLAGS "--closure=1 ")
         add_compile_definitions(PSP_WASM_OPFS_PERSIST=1)
-    else()
-        set(PSP_FILESYSTEM_FLAGS "-s NO_FILESYSTEM=1 ")
-        set(PSP_CLOSURE_FLAGS "--closure=1 ")
     endif()
 
     set(PSP_WASM_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} \

rust/perspective-server/cpp/perspective/src/cpp/arrow_loader.cpp

@@ -934,9 +934,7 @@ ArrowLoader::fill_column(
         std::int64_t null_count = array->null_count();
 
         if (null_count == 0) {
-            for (uint32_t i = 0; i < len; ++i) {
-                col->set_valid(offset + i, true);
-            }
+            col->set_valid_range(offset, len);
         } else {
             const uint8_t* null_bitmap = array->null_bitmap_data();
 

rust/perspective-server/cpp/perspective/src/cpp/column.cpp

@@ -620,6 +620,17 @@ t_column::set_status(t_uindex idx, t_status status) {
     m_status->set_nth<t_status>(idx, status);
 }
 
+void
+t_column::set_valid_range(t_uindex offset, t_uindex len) {
+    if (!is_status_enabled() || len == 0) {
+        return;
+    }
+    static_assert(
+        sizeof(t_status) == 1, "set_valid_range assumes a 1-byte t_status"
+    );
+    std::memset(m_status->get_nth<t_status>(offset), STATUS_VALID, len);
+}
+
 void
 t_column::set_scalar(t_uindex idx, t_tscalar value) {
     COLUMN_CHECK_ACCESS(idx);

rust/perspective-server/cpp/perspective/src/cpp/computed_expression.cpp

@@ -102,6 +102,27 @@ t_computed_expression::t_computed_expression(
     m_column_ids(column_ids),
     m_dtype(dtype) {}
 
+struct t_computed_expression_cache {
+    t_computed_expression_cache() = default;
+    t_computed_expression_cache(const t_computed_expression_cache&) = delete;
+    t_computed_expression_cache& operator=(const t_computed_expression_cache&) =
+        delete;
+    t_computed_expression_cache(t_computed_expression_cache&&) = delete;
+    t_computed_expression_cache& operator=(t_computed_expression_cache&&) =
+        delete;
+
+    std::shared_ptr<t_data_table> m_current_source_table;
+    t_uindex m_row_idx{0};
+    std::vector<std::pair<std::string, t_tscalar>> m_values;
+    std::vector<std::shared_ptr<t_column>> m_columns;
+    std::vector<t_dtype> m_input_dtypes;
+    exprtk::symbol_table<t_tscalar> m_sym_table;
+    exprtk::expression<t_tscalar> m_expr;
+    std::unique_ptr<t_computed_function_store> m_function_store;
+};
+
+t_computed_expression::~t_computed_expression() = default;
+
 void
 t_computed_expression::compute(
     const std::shared_ptr<t_data_table>& source_table,
@@ -110,53 +131,94 @@ t_computed_expression::compute(
     t_expression_vocab& vocab,
     t_regex_mapping& regex_mapping
 ) const {
-    // TODO: share symtables across pre/re/compute
-    exprtk::symbol_table<t_tscalar> sym_table;
+    auto num_input_columns = m_column_ids.size();
 
-    // pi, infinity, etc.
-    sym_table.add_constants();
+    bool needs_build = !m_cache;
+    if (!needs_build) {
+        for (t_uindex cidx = 0; cidx < num_input_columns; ++cidx) {
+            const std::string& column_name = m_column_ids[cidx].second;
+            if (source_table->get_column(column_name)->get_dtype()
+                != m_cache->m_input_dtypes[cidx]) {
+                needs_build = true;
+                break;
+            }
+        }
+    }
 
-    t_uindex row_idx = 0;
+    if (needs_build) {
+        m_cache = std::make_unique<t_computed_expression_cache>();
+        auto& cache = *m_cache;
+
+        cache.m_current_source_table = source_table;
+        cache.m_sym_table.add_constants(); // pi, infinity, etc.
+
+        // is_type_validator = false: we are computing values, not type-checking.
+        cache.m_function_store = std::make_unique<t_computed_function_store>(
+            vocab,
+            regex_mapping,
+            false,
+            cache.m_current_source_table,
+            pkey_map,
+            cache.m_row_idx
+        );
+        cache.m_function_store->register_computed_functions(cache.m_sym_table);
 
-    // Create a function store, with is_type_validator set to false as we
-    // are calculating values, not type-checking.
-    t_computed_function_store function_store(
-        vocab, regex_mapping, false, source_table, pkey_map, row_idx
-    );
-    function_store.register_computed_functions(sym_table);
+        // Size exactly once; the symbol table binds a T* into each
+        // m_values[cidx].second, so this storage must never be reallocated.
+        cache.m_values.resize(num_input_columns);
+        cache.m_columns.resize(num_input_columns);
+        cache.m_input_dtypes.resize(num_input_columns);
 
-    exprtk::expression<t_tscalar> expr_definition;
-    std::vector<std::pair<std::string, t_tscalar>> values;
-    tsl::hopscotch_map<std::string, std::shared_ptr<t_column>> columns;
+        for (t_uindex cidx = 0; cidx < num_input_columns; ++cidx) {
+            const std::string& column_id = m_column_ids[cidx].first;
+            const std::string& column_name = m_column_ids[cidx].second;
+            t_dtype dtype = source_table->get_column(column_name)->get_dtype();
 
-    auto num_input_columns = m_column_ids.size();
-    values.resize(num_input_columns);
-    columns.reserve(num_input_columns);
+            cache.m_input_dtypes[cidx] = dtype;
 
-    for (t_uindex cidx = 0; cidx < num_input_columns; ++cidx) {
-        const std::string& column_id = m_column_ids[cidx].first;
-        const std::string& column_name = m_column_ids[cidx].second;
-        columns[column_id] = source_table->get_column(column_name);
+            t_tscalar rval;
+            rval.clear();
+            rval.m_type = dtype;
 
-        t_tscalar rval;
-        rval.clear();
-        rval.m_type = columns[column_id]->get_dtype();
+            cache.m_values[cidx] =
+                std::pair<std::string, t_tscalar>(column_id, rval);
+            cache.m_sym_table.add_variable(
+                column_id, cache.m_values[cidx].second
+            );
+        }
 
-        values[cidx] = std::pair<std::string, t_tscalar>(column_id, rval);
-        sym_table.add_variable(column_id, values[cidx].second);
+        cache.m_expr.register_symbol_table(cache.m_sym_table);
+
+        // Load-bearing for compile-once: exprtk constant-folds an all-literal
+        // function call (e.g. intern('x'), concat('a','b')) to a literal node at
+        // compile time ONLY when the function reports no side effects. Our
+        // functions inherit the igeneric_function/ifunction default
+        // has_side_effects() == true and never call disable_has_side_effects(),
+        // so such calls are not folded and re-evaluate on every value(). If a
+        // future exprtk bump flips that default, the first call's result would
+        // be frozen into the cached AST -- revisit this cache if so.
+        if (!m_computed_expression_parser.m_parser->compile(
+                m_parsed_expression_string, cache.m_expr
+            )) {
+            std::stringstream ss;
+            ss << "[t_computed_expression::compute] Failed to parse "
+                  "expression: `"
+               << m_parsed_expression_string << "`, failed with error: "
+               << m_computed_expression_parser.m_parser->error() << '\n';
+
+            PSP_COMPLAIN_AND_ABORT(ss.str());
+        }
     }
 
-    expr_definition.register_symbol_table(sym_table);
-
-    if (!m_computed_expression_parser.m_parser->compile(
-            m_parsed_expression_string, expr_definition
-        )) {
-        std::stringstream ss;
-        ss << "[t_computed_expression::compute] Failed to parse expression: `"
-           << m_parsed_expression_string << "`, failed with error: "
-           << m_computed_expression_parser.m_parser->error() << '\n';
+    auto& cache = *m_cache;
 
-        PSP_COMPLAIN_AND_ABORT(ss.str());
+    // Re-point the per-call inputs the compiled function objects read through
+    // (the source table differs across the master/flattened/delta/prev/current
+    // tables within a single update), and re-fetch this call's source columns.
+    cache.m_current_source_table = source_table;
+    for (t_uindex cidx = 0; cidx < num_input_columns; ++cidx) {
+        cache.m_columns[cidx] =
+            source_table->get_column(m_column_ids[cidx].second);
     }
 
     // create or get output column using m_expression_alias
@@ -167,12 +229,13 @@ t_computed_expression::compute(
 
     for (t_uindex ridx = 0; ridx < num_rows; ++ridx) {
         for (t_uindex cidx = 0; cidx < num_input_columns; ++cidx) {
-            const std::string& column_id = m_column_ids[cidx].first;
-            values[cidx].second.set(columns[column_id]->get_scalar(ridx));
+            cache.m_values[cidx].second.set(
+                cache.m_columns[cidx]->get_scalar(ridx)
+            );
         }
-        row_idx = ridx;
+        cache.m_row_idx = ridx;
 
-        t_tscalar value = expr_definition.value();
+        t_tscalar value = cache.m_expr.value();
 
         if (!value.is_valid() || value.is_none()) {
             output_column->clear(ridx);
@@ -182,7 +245,8 @@ t_computed_expression::compute(
         output_column->set_scalar(ridx, value);
     }
 
-    function_store.clear_computed_function_state();
+    // order()'s accumulator must still be reset at the end of every call.
+    cache.m_function_store->clear_computed_function_state();
 };
 
 const std::string&

rust/perspective-server/cpp/perspective/src/cpp/computed_function.cpp

@@ -2330,12 +2330,12 @@ make_datetime::operator()(t_parameter_list parameters) {
 
 index::index(
     const t_gstate::t_mapping& pkey_map,
-    std::shared_ptr<t_data_table> source_table,
+    const std::shared_ptr<t_data_table>& source_table,
     t_uindex& row_idx
 ) :
     exprtk::igeneric_function<t_tscalar>("Z"),
     m_pkey_map(pkey_map),
-    m_source_table(std::move(std::move(source_table))),
+    m_source_table(source_table),
     m_row_idx(row_idx) {}
 
 index::~index() = default;
@@ -2355,13 +2355,13 @@ index::operator()(t_parameter_list parameters) {
 col::col(
     t_expression_vocab& expression_vocab,
     bool is_type_validator,
-    std::shared_ptr<t_data_table> source_table,
+    const std::shared_ptr<t_data_table>& source_table,
     t_uindex& row_idx
 ) :
     exprtk::igeneric_function<t_tscalar>("T"),
     m_expression_vocab(expression_vocab),
     m_is_type_validator(is_type_validator),
-    m_source_table(std::move(std::move(source_table))),
+    m_source_table(source_table),
     m_row_idx(row_idx) {}
 col::~col() = default;
 
@@ -2394,13 +2394,13 @@ col::operator()(t_parameter_list parameters) {
 vlookup::vlookup(
     t_expression_vocab& expression_vocab,
     bool is_type_validator,
-    std::shared_ptr<t_data_table> source_table,
+    const std::shared_ptr<t_data_table>& source_table,
     t_uindex& row_idx
 ) :
     exprtk::igeneric_function<t_tscalar>("TT"),
     m_expression_vocab(expression_vocab),
     m_is_type_validator(is_type_validator),
-    m_source_table(std::move(std::move(source_table))),
+    m_source_table(source_table),
     m_row_idx(row_idx) {}
 vlookup::~vlookup() = default;
 

rust/perspective-server/cpp/perspective/src/cpp/context_grouped_pkey.cpp

@@ -152,12 +152,12 @@ t_ctx_grouped_pkey::get_data(
     }
 
     auto* aggtable = m_tree->get_aggtable();
-    t_schema aggschema = aggtable->get_schema();
 
     for (t_uindex aggidx = 0, loop_end = aggcols.size(); aggidx < loop_end;
          ++aggidx) {
-        const std::string& aggname = aggschema.m_columns[aggidx];
-        aggcols[aggidx] = aggtable->_get_const_column(aggname);
+        // resolve by column index (== aggidx); skips the per-iteration
+        // name->index map lookup + temp string and the schema deep-copy.
+        aggcols[aggidx] = aggtable->_get_const_column(aggidx);
     }
 
     const std::vector<t_aggspec>& aggspecs = m_config.get_aggregates();