digital-fabric / extralite

Ruby on SQLite
http://www.rubydoc.info/gems/extralite
MIT License
255 stars 8 forks source link

Implement parameter transform #45

Closed noteflakes closed 10 months ago

noteflakes commented 11 months ago

This PR adds the ability to define a parameter transform in order to support providing arbitrary parameters of any type, as an alternative to #33.

gschlager commented 11 months ago

Thanks! That looks a lot cleaner.

Unfortunately it's also a lot slower.

# frozen_string_literal: true

require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'
  gem 'benchmark-ips'
end

require 'benchmark/ips'
require 'date'
require_relative '../lib/extralite'

db = Extralite::Database.new(":memory:")
db_with_transform = Extralite::Database.new(":memory:")
db_with_transform.parameter_transform = ->(v) do
  case v
  when Time
    v.to_i
  when Date
    v.to_s
  when Hash
    v.to_json
  else
    v
  end
end

sql = "SELECT ?, ?, ?, ?, ?, ?, ?, ?, ?, ?"
stmt = db.prepare(sql)
stmt_with_transform = db_with_transform.prepare(sql)

the_time = Time.now

Benchmark.ips do |x|
  x.config(:time => 3, :warmup => 1)

  x.report("regular") { stmt.bind("foo", the_time.to_i, 32, 82.02, "bar", true, false, nil, 42, 393) }
  x.report("parameter_transform") { stmt_with_transform.bind("foo", the_time, 32, 82.02, "bar", true, false, nil, 42, 393) }

  x.compare!
end
ruby 3.2.2 (2023-03-30 revision e51014f9c0) [x86_64-linux]
Warming up --------------------------------------
             regular   261.952k i/100ms
 parameter_transform    86.131k i/100ms
Calculating -------------------------------------
             regular      2.727M (± 0.4%) i/s -      8.382M in   3.074279s
 parameter_transform    876.001k (± 0.2%) i/s -      2.670M in   3.048026s

Comparison:
             regular:  2726691.4 i/s
 parameter_transform:   876000.6 i/s - 3.11x  slower

If we optimize parameter_transform so that it ignores all types that are already implemented then it's not as slow anymore.

static inline VALUE parameter_transform(VALUE block, VALUE value) {
  if (NIL_P(block))
    return value;

  switch (TYPE(value)) {
    case T_NIL:
    case T_FIXNUM:
    case T_BIGNUM:
    case T_FLOAT:
    case T_TRUE:
    case T_FALSE:
    case T_SYMBOL:
    case T_STRING:
      return value;
    default:
      return CALL_BLOCK(block, value);
  }
}
ruby 3.2.2 (2023-03-30 revision e51014f9c0) [x86_64-linux]
Warming up --------------------------------------
             regular   264.679k i/100ms
 parameter_transform   225.107k i/100ms
Calculating -------------------------------------
             regular      2.668M (± 0.5%) i/s -      8.205M in   3.075133s
 parameter_transform      2.220M (± 0.6%) i/s -      6.753M in   3.041919s

Comparison:
             regular:  2668262.6 i/s
 parameter_transform:  2220123.8 i/s - 1.20x  slower

Is there really a good use case for transforming all parameters? Is it worth the slowdown? I can live with 1.20x slower, but 3.11x slower is too much.

gschlager commented 11 months ago

I need to think about this some more. Named parameters are already quite slow compared to regular ? parameters. Adding even more functionality into the parameter binding pipeline isn't helping. I need to do more benchmarking.

noteflakes commented 10 months ago

I'm closing this for the moment as the performance costs are not satisfactory, and it is also not clear that the benefits are important enough. We might revisit at a later occasion.