rails/jbuilder — pull request #54 by rolftimmermans (2012-09-05 15:54:28+00:00)
Title: Substantial performance improvements

Hi,

Jbuilder is awesome. However, it can be a little slow if the JSON that is built is very large. This patch aims to improve the situation.

Before I start explaining the changes, let me stress to anyone reading this that it is important to use a fast JSON serialisation library, such as `oj`. Please use `oj` before considering this patch an improvement. Compared to slower libraries it may yield a 10x performance improvement during serialisation.

The baseline benchmark that I used can be found here: https://gist.github.com/3638182 It is a stand-alone method that serialises an object with a couple of attributes and 2 arrays with 100 children each. Each child has 2-4 attributes.

Baseline benchmark (using `oj` as serialisation library):

```
without key_format
user system total real
build 6.720000 0.040000 6.760000 ( 6.773519)

with Jbuilder.key_format :camelize => :lower
user system total real
build 38.680000 0.220000 38.900000 ( 39.039534)
```

Benchmark with the changes in this pull request:

```
without key_format
user system total real
build 3.260000 0.020000 3.280000 ( 3.281165)

with Jbuilder.key_format :camelize => :lower
user system total real
build 3.820000 0.020000 3.840000 ( 3.841956)
```

About twice as fast without `Jbuilder.key_format`. When using `Jbuilder.key_format :camelize => :lower` the improvements were almost 10x. When used with Rails the performance improvements will be diluted, but could still be significant.

Here is a summary of the performance-related changes/commits and their cumulative effect on the benchmark without key formatting:

```
Avoid calls to method_missing by calling set! (5.631219)
Refactor method_missing to use fewer comparisons. (4.949613)
Avoid many calls to Array#first. (4.438711)
Set value directly, avoiding calls to set! (4.376998)
Avoid creating lambda objects. (4.120978)
Key formatter should cache formatted keys. (3.463397)
Avoid calls to respond_to? when we really need Module#===. (3.371929)
Simplify array! to avoid calls to child! (3.281165)
```

Most changes are quite small, but there is one significant change that I'd like to discuss. It is the change that leads to the enormous difference when using a key formatter.

Previously, each JSON key was always formatted when used. When iterating over many children, this would result in a lot of duplicate effort. Therefore, I've introduced a `KeyFormatter` object that will cache known keys and only perform (possibly expensive) formatting if the key isn't known. A new `KeyFormatter` is instantiated for each root `Jbuilder` object, and reused for child objects.

As currently implemented however, it leads to a subtle change in functionality. Previously, it was possible to expand on previously set key formats:

```
json.key_format! :upcase
json.child do |json|
# The key will be "VALUE"
json.value "two"
end
```
```
json.key_format! :upcase
json.child do |json|
json.key_format! ->(key) {key + " foo"}
# The key will be "VALUE foo"
json.value "two"
end
```

Except it only worked like that when using `Jbuilder` directly, i.e. outside a Rails `jbuilder` template. Inside Rails, the `key_format` was not even propagated to child elements:

```
json.key_format! :upcase
json.child do |json|
# The key will be "value"
json.value "two"
end
```
```
json.key_format! :upcase
json.child do |json|
json.key_format! ->(key) {key + " foo"}
# The key will be "value foo"
json.value "two"
end
```

This pull request also fixes the fact that the key format is not propagated in Rails (and adding a few test for the `JbuilderTemplate` as used with Rails). However, I did not add the second behaviour: the ability to 'stack' multiple key formatters on top of each other. It should be possible to add such a feature relatively easy, however.

I fully realise this pull request has a many changes in it, and that the slightly different behaviour with regards to key formatting needs to be considered carefully.

Please let me know your opinion(s) on these changes. I would be quite happy to adjust these patches if that means some or all of it can be merged.