artsy/garner — pull request #39 by fancyremarker (2013-06-27 17:40:48+00:00)
Title: BindingIndex strategy pair

to: @joeyAghion

(The implementation is in the second commit, 47d50bb69d2a2b4c752f746963c61c73a9773950.)

### The Good

* This implements, as `Strategies::Binding::Key::BindingIndex` and `Strategies::Binding::Invalidation::BindingIndex`, a binding key/invalidation strategy modeled after Garner 0.3.3's implementation, providing a better guarantee of similar performance for all clients to earlier Garner implementations, and also, [**choices**](http://en.wikipedia.org/wiki/The_Paradox_of_Choice:_Why_More_Is_Less). This is America, people.
* Performance: This strategy pair performs better than `SafeCacheKey` / `Touch` on every aspect except soft invalidation (i.e., invalidation via callbacks). Using the same benchmarking parameters as in #34 (i.e., on MongoHQ, a Heroku memcached server, etc.):

```
$ N=100 D=12850 R=8 bundle exec rake benchmark

Key: Base
Invalidation: Base
MethodProfiler results for: BenchmarkContext
+-----------------------+-----------+-----------+--------------+-------------+
| Method | Min Time | Max Time | Average Time | Total Time |
+-----------------------+-----------+-----------+--------------+-------------+
| #a_warm_virtual_fetch | 18.688 ms | 57.297 ms | 28.601 ms | 2860.062 ms |
| #b_cold_virtual_fetch | 19.437 ms | 56.924 ms | 29.639 ms | 2963.948 ms |
| #c_warm_class_fetch | 18.770 ms | 63.080 ms | 27.743 ms | 2774.345 ms |
| #d_cold_class_fetch | 19.444 ms | 63.607 ms | 28.753 ms | 2875.317 ms |
| #e_force_invalidate | 0.013 ms | 0.055 ms | 0.023 ms | 2.345 ms |
| #f_soft_invalidate | 0.015 ms | 0.036 ms | 0.025 ms | 2.549 ms |
+-----------------------+-----------+-----------+--------------+-------------+

Key: SafeCacheKey
Invalidation: Touch
MethodProfiler results for: BenchmarkContext
+-----------------------+-----------+-----------+--------------+-------------+
| Method | Min Time | Max Time | Average Time | Total Time |
+-----------------------+-----------+-----------+--------------+-------------+
| #a_warm_virtual_fetch | 2.899 ms | 42.385 ms | 4.604 ms | 460.361 ms |
| #b_cold_virtual_fetch | 24.035 ms | 65.943 ms | 36.919 ms | 3691.924 ms |
| #c_warm_class_fetch | 2.650 ms | 36.047 ms | 4.418 ms | 441.820 ms |
| #d_cold_class_fetch | 23.473 ms | 65.135 ms | 37.535 ms | 3753.493 ms |
| #e_force_invalidate | 0.714 ms | 2.137 ms | 1.196 ms | 119.572 ms |
| #f_soft_invalidate | 0.016 ms | 0.073 ms | 0.028 ms | 2.848 ms |
+-----------------------+-----------+-----------+--------------+-------------+

Key: BindingIndex
Invalidation: BindingIndex
MethodProfiler results for: BenchmarkContext
+-----------------------+-----------+-----------+--------------+-------------+
| Method | Min Time | Max Time | Average Time | Total Time |
+-----------------------+-----------+-----------+--------------+-------------+
| #a_warm_virtual_fetch | 2.436 ms | 34.913 ms | 3.181 ms | 318.052 ms |
| #b_cold_virtual_fetch | 23.700 ms | 71.570 ms | 36.134 ms | 3613.434 ms |
| #c_warm_class_fetch | 1.845 ms | 35.495 ms | 4.186 ms | 418.592 ms |
| #d_cold_class_fetch | 22.937 ms | 64.659 ms | 35.878 ms | 3587.850 ms |
| #e_force_invalidate | 0.844 ms | 5.824 ms | 1.139 ms | 113.906 ms |
| #f_soft_invalidate | 0.885 ms | 28.651 ms | 1.372 ms | 137.246 ms |
+-----------------------+-----------+-----------+--------------+-------------+
```

### The Bad

* This leaves partially unresolved your concern raised in #38. While this does provide a `touch`-free alternative invalidation strategy, it's more complicated than the `SafeCacheKey` strategy. I plan to address this in my next pull request by modifying the strategy configuration to allow using multiple strategies in parallel. Specifically:
* `:binding_key_strategy`: The strategy to be used for object instance bindings.
* `:binding_key_class_strategies`: If specified, the strategy to be used for class bindings instead of `:binding_key_strategy`.
* `:binding_invalidation_strategies`: An array of invalidation strategies, all of which will be applied, in order.

### The Ugly

* This pull request introduces a strange symmetry between `proxy_binding` and `canonical?`; If not `canonical?` an object must implement `proxy_binding` and its proxy binding must be canonical, in order to generate a cache key. I don't like this implicit contract, but I also don't think it's too dangerous, because if the symmetry is broken, it just means more cache misses, not loss of consistency.
* The implementation of `canonical?` sucks. This is related to the first bullet point. I couldn't think of a great solution, but would appreciate ideas.
* Class bindings are now treated specially for the first time (see `proxied_classes`, e.g.) Maybe this isn't so bad, because the implication of binding to "any instance of a class" seems intrinsically different than binding to a specific instance, but if so, I wish this was more explicit.