Specific interfaces – in the small

Wednesday, March 7, 2012

Everyone on the Web I found who states that quote I was looking for says “I don’t know who said it, but be ‘Generous on input, strict on output’” (or some variation on this). While I am unsure about the first proposition, I wholeheartedly agree with the second.

Edit 3/8: the quote is known in a different wording as Postel’s Law, which shows up as the Robustness Principle in RFC 793, the specification of TCP. Thanks for the hint, Austin!

Unfortunately, the closest a Rubyist typically gets to the implementation of an interface specification is his tests. This provides a pretty good, but somewhat disconnected specification that can sometimes cover up imprecisions in the interface’s implementation.

On top of that, sometimes our frameworks make it easy to forget what our tests are asserting or spec’ing.

Take rspec’s predicate matchers and this example:

require 'rspec/core'

class VeryImportantQuestions
  def self.really?(answer)
    answer == 'Yes. I am telling you.'
  end

  def self.really_really?(answer)
    answer == 'Yes. I am telling you.' ? 42 : nil
  end
end

describe "really?" do
  context "using rspec predicate matchers" do
    context "if someone is telling you" do
      it "should be really really the case and return true" do
        VeryImportantQuestions.really?('Yes. I am telling you.').should be_true
      end
    end
    context "if someone is not sure" do
      it "should return false" do
        VeryImportantQuestions.really?('I am not sure.').should be_false
      end
    end
  end
end

describe "really_really?" do
  context "using rspec predicate matchers" do
    context "if someone is telling you" do
      it "should be really really the case and return true" do
        VeryImportantQuestions.really_really?('Yes. I am telling you.').should be_true
      end
    end
    context "if someone is not sure" do
      it "should return false" do
        VeryImportantQuestions.really_really?('I am not sure.').should be_false
      end
    end
  end
end

be_true and be_false effectively hide the fact that what’s actually spec’ed is truthiness and falsiness. Only when the following context is added is this imprecision revealed:

  context "spec'ing the actual output of the method fails" do
    context "if someone is telling you" do
      it "should be really really the case and return true" do
        VeryImportantQuestions.really_really?('Yes. I am telling you.').should == true
      end
    end
    context "if someone is not sure" do
      it "should return false" do
        VeryImportantQuestions.really_really?('I am not sure.').should == false
      end
    end
  end

With regard to rspec, I suggest to consider twice whether the benefits of using specific matchers to not outweigh their benefits in your situation. You might get nicer test output, but you might lose the ability to immediately tell what you’re spec’ing.

With regard to tests in general: be specific about what you output – aka be specific about what you test.

Here is the gist: https://gist.github.com/1998462

SF Standup – Feb 13, 2012 – The Sound of One Hand

Ken Mayer

Monday, February 13, 2012

Cries for help

Capybara is reporting “stale elements” for no good reason.
Any experience, war stories upgrading from jQuery 1.6 to 1.7, especially with the whole event delegation API change? [crickets] See interestings…

Interestings

Have a really big spec file that takes a long time to run? http://rubygems.org/gems/parallel_split_test, from our own Michael Grosser, will run a big spec file in smaller chunks.
url helpers cache results for better performance, but they seem to be losing custom options (in this case a locales setting).
In jQuery 1.6, triggering an event in an unattached DOM node would propagate the event up into the document, even though it was never attached to it. jQuery 1.7 (maybe 1.7.1) fixes this behavior.

Test your Rake tasks!

Stephan Hagemann

Sunday, February 5, 2012

There are several reasons why you should test your Rake tasks:

Rake tasks are code and as such deserve testing.
When untested Rake tasks have a tendency to become overly long and convoluted. Tests will help keep them in bay.
As Rake tasks typically depend on your models, you (should) loose confidence in them if you don’t have tests and are attempting refactorings.

A problematic Rake task test

Here is a Rake file…

File: lib/tasks/bar_problematic.rake

namespace :foo do
  desc "bake some bars"
  task bake_a_problematic_bar: :environment do
    puts '*' * 60
    puts ' Step back: baking in action!'
    puts '*' * 60

    puts Bar.new.bake

    puts '*' * 60
    puts ' All done. Thank you for your patience.'
    puts '*' * 60
  end
end

…and its too simplistic spec:

File: spec/tasks/bar_rake_problematic_spec.rb
require 'spec_helper'
require 'rake'

describe 'foo namespace rake task' do
  describe 'foo:bake_a_problematic_bar' do

    before do
      load File.expand_path("../../../lib/tasks/bar_problematic.rake", __FILE__)
      Rake::Task.define_task(:environment)
    end

    it "should bake a bar" do
      Bar.any_instance.should_receive :bake
      Rake::Task["foo:bake_a_problematic_bar"].invoke
    end

    it "should bake a bar again" do
      Bar.any_instance.should_receive :bake
      Rake::Task["foo:bake_a_problematic_bar"].invoke
    end
  end
end

Some notable aspects of testing Rake tasks:

Rake has to be required.
The Rake file under test has to be manually loaded.
In this example, the Rake task depends on the environment task, which is not automatically available in a spec. Since we are in rspec, the environment is already loaded and we can just define environment as an empty Rake task to make the bake task run in the test.

When run, this spec fails on the second it block… and that is not the only problem with this spec and the Rake task:

The Rake task duplicates code to output information to the user.
The spec “should bake a bar” will output that information when run, which clobbers the spec runners output.
The spec “should bake a bar” again will fail, because Rake tasks are built to only execute once per process. See rake.rb. This makes sense for the normal use of Rake tasks where a task may be named as the prerequisite of another task multiple times through multiple dependencies it might have – the task only needs to run once. In our tests we have to reenable the task.

A better Rake task test

A new version of the above Rake file…

File: lib/tasks/bar.rake
class BarOutput
  def self.banner text
    puts '*' * 60
    puts " #{text}"
    puts '*' * 60
  end

  def self.puts string
    puts string
  end
end

namespace :foo do
  desc "bake some bars"
  task bake_a_bar: :environment do
    BarOutput.banner " Step back: baking in action!"
    BarOutput.puts Bar.new.bake
    BarOutput.banner " All done. Thank you for your patience."
  end
end

… and its spec:

File: spec/tasks/bar_rake_spec.rb
require 'spec_helper'
require 'rake'

describe 'foo namespace rake task' do
  before :all do
    Rake.application.rake_require "tasks/bar"
    Rake::Task.define_task(:environment)
  end

  describe 'foo:bar' do
    before do
      BarOutput.stub(:banner)
      BarOutput.stub(:puts)
    end

    let :run_rake_task do
      Rake::Task["foo:bake_a_bar"].reenable
      Rake.application.invoke_task "foo:bake_a_bar"
    end

    it "should bake a bar" do
      Bar.any_instance.should_receive :bake
      run_rake_task
    end

    it "should bake a bar again" do
      Bar.any_instance.should_receive :bake
      run_rake_task
    end

    it "should output two banners" do
      BarOutput.should_receive(:banner).twice
      run_rake_task
    end

  end
end

This spec passes just fine and does not clobber the spec output. Again, let’s look at noteworthy things:

The output of the Rake task now goes through the BarOutput class. This reduces code duplication and allows for easy stubbing. There are other ways to achieve a similar effect and not clobber test output: Stub puts and print, stub on $stdout.
Rake.application has a nicer way of requiring Rake files than a simple load, because rake_require knows where Rake files live.
Rake::Task["TASK"].reenable reenables the task with name “TASK” so that it will be run again and can be called multiple times in a spec.

Here is the gist: https://gist.github.com/1764423

Standup 2012/2/1: Speed kills

Jacob Maine

Wednesday, February 1, 2012

Interesting Things

If you haven’t noticed, Jasmine tests are at least twice as fast in Chrome as they are in Firefox. Closing the inspection pane makes it even faster. Be aware that part of the speed is from Chrome’s aggressive caching, which can lead to erroneous test results.
One team is using Backbone’s local storage. When they add model.clear() after every test run, their tests go from 20 seconds to over 100 seconds. Someone suggested the silent: true option, to suppress the change events that clear triggers.
To avoid bugs in minified JS put semicolons in the right spots. The easiest way to do that is to run a tool like JSLint or JSHint over your code. Add it to your test suite to prevent mistakes.

Ask for Help

“In IE8 the numbers don’t show up on ordered lists if we dynamically create lis”

Or rather, they do, but only after hovering over the list. The common wisdom is that this has been broken in IE for a long time.

“Our project does some DNS resolution. Is there a preferred way to mock this in tests?”

No suggestions.

“When replying to an email each email system adds different junk to the message. We’re processing those incoming replies. Any standard way to strip out the junk?”

Everyone is using the ugly regex approach. Are there mail gems that handle this?

Standup 2012/1/31: The bleeding edge

Jacob Maine

Tuesday, January 31, 2012

Interesting Things

There’s a new release of Backbone – 0.9.0. It’s billed as a release candidate for 1.0, and seems to be a bit buggy, as RCs can be. However, it’s exciting to see that Backbone is getting close to that milestone.
You should default boolean fields to true or false, at the database layer. Otherwise your queries have to deal with three-valued logic.
Rails 3.2 has some unexpected behavior. First, the generated form ids have changed … what used to be id=user_new is now id=new_user. Second, if your routes file is missing an entry, you will no longer get errors in controller tests. If you liked that behavior, try out request specs.

Ask for Help

“Anyone seen problems with the latest REE and iconv?”

Everything works on the Linux machines, but on Macs, there’s an error about an unrecognized target encoding. Iconv works on the command line, so it’s something about REE.

Standup 1/30/2012: It’s all about sharing

Jacob Maine

Monday, January 30, 2012

Interesting Things

should_not render_with_layout can be flaky. For example, asserting that an XHR request doesn’t have a layout could fail if the request renders an email template that does have a layout. These matchers are not very sophisticated, mostly doing string matching, not real template resolution.

Ask for Help

“How do I test relative time with FixtureBuilder?”

Since the models are all built beforehand, it’s tricky to write tests that make assertions about relative time. There were a variety of suggestions. You can write all the tests relative to a model: some_time.should == model.created_at - 3.days. You can wrap the entire suite and fixture generation in a Timecop block, but you’ll have to deal with losing that context whenever any test calls Timecop.return.

“How do I separate multiple FixtureBuilder files, but allow them to reference objects created in each other?”

Typically you want to separate fixtures into smaller files. But, you also often want to reference objects in other files, for example so you can assign a user from user_fixtures.rb to a post in post_fixtures.rb. One solution is to eval each file in a loop, so they can share instance variables. You can also use User.where(:name => "Tom") but that’s awkward and repetitive.

“Any suggestions for a CMS that would allow lots of different visual presentations of the content?”

The questioner is experimenting with branding. Not many suggestions… a shared database perhaps?

Standup 11/11/2011: Some funkinesses

Stephan Hagemann

Friday, November 11, 2011

Interesting

rspec stub != stub!. stub! is an alias method for stub. There is however also a method stub that is an alias for double. If you try to stub a method on the test class (to stub it on the context), you should probably use the magic subject/helper/controller methods. If you don’t, using self.stub(:name => 'result') will create a double, while self.stub!(:name => 'result') will stub the method as you would expect.
Asynchronous file creation and downloading: if an asynchronous process writes a file using File.open and f.write, an other process checking the presence of the file to determine whether it is already available for download, will deliver the empty file, if the file has been opened, but not yet written.
- Workarounds:
  - if you have one write to the file only: check filesize.
  - update an ActiveRecord attribute after the file writing is completed and check against that.
== on DelegateClass: newing up an instance delegate_x of DelegateClass from object x, x == delegate_x, while of course x.class != delegate_x.class.

Keystroke of the day

Rubymine KOTD: The search+replace mode you reach via Cmd+r allows you to see recent searches by hitting the down arrow. If that doesn’t work for you in Lion, hit Ctrl+h.

Cedar Expectations

Adam Milligan

Wednesday, November 2, 2011

As I wrote here we’ve used OCHamcrest matchers for some time for writing expectations in Cedar, but have found them unsatisfying. We wanted convenient matchers, like the ones Jasmine provides for JavaScript, but for Objective C. To that end, we added Cedar-specific expectation functions and matchers that specifically solve the problems we had with OCHamcrest.

To use Cedar’s expectations you need to make a couple small changes to your spec files:

1) Cedar matchers use C++ templates. Tell the compiler to expect some C++ code by changing the file extension on your spec files from .m to .mm.

2) Cedar matchers live in a C++ namespace. At the top of your spec file, after the includes, add this line:

using namespace Cedar::Matchers;

Features

Type Deduction

Cedar matchers determine the types of the values in the expectation, so you don’t have to. Rather than this:

assertThat(someObject, equalTo(anotherObject));
assertThatUnsignedInt(someUInt, equalToUnsignedInt(anotherUInt));

you write this:

expect(someObject).to(equal(anotherObject));
expect(someUInt).to(equal(anotherUInt));

We can also mix and match object and non-object types, in cases where such comparisons make sense. For instance, rather than this:

NSNumber *aNumber = [NSNumber numberWithFloat:1.7];
assertThat(aNumber, equalTo([NSNumber numberWithFloat:1.8));
assertThatFloat([aNumber floatValue], equalToFloat(1.8));  // equivalent

you write this:

NSNumber *aNumber = [NSNumber numberWithFloat:1.7];
expect(aNumber).to(equal(1.8));

Extensibilty

Cedar’s expectations use C++ templates not just for argument types, but for matcher objects as well. This means what goes inside the to() construct (e.g. expect(foo).to(equal(bar));) can be any object that responds to the appropriate methods (more on this later). If you want to write a custom matcher for your system (e.g. expect(user).to(be_logged_in());), no need to subclass anything or link against the Cedar library.

We also separated comparators from matchers for common expectations, such as equality. This means that adding equality comparison for a custom type involves only adding a comparator function for your type, with no need to modify the equality matcher itself. For example, here is the method for comparing two NSNumber objects:

bool compare_equal(NSNumber * const actualValue, NSNumber * const expectedValue) {
    return [actualValue isEqualToNumber:expectedValue];
}

Caveats

Several versions of LLVM have had trouble with properly compiling Objective C++ code that contains blocks, and GCC has never done so correctly. Versions of LLVM before 2.0 (Xcode 3.x) will fail to compile Cedar specs with the Internal Compiler Error” message. Version 2.0 of LLVM (Xcode 4.0.x) will successfully compile Cedar specs, but incorrectly handle reference to temporaries, leading to difficult to understand error messages. Versions 2.1 (Xcode 4.1) and 3.0 (Xcode 4.2) fix this problem, and will work properly. However, most version of Xcode still default to using the GCC/LLVM hybrid, so you may need to change this setting.

Also, C++ templates deduce types at compile time, which means they use the matcher function for the declared type of parameters. For instance, if you have an equality comparator specialized for MyType, which is a subclass of NSObject, the following code will invoke the equality comparator for NSObject, not MyType:

NSObject *foo = [[MyType alloc] init]; // refers to a MyType, but is declared as NSObject *
expect(foo).to(equal(someOtherObject));

In practice this seems to cause problems less frequently than I had expected. However, keep in mind that the declared type of nearly all Objective C initializers is id (NSNumber seems to be the exception to this rule, for some reason). So, if you pass the temporary result of an initializer to a matcher, it will most likely use the specialization of that matcher for id. For instance:

expect([NSString string]).to(equal(@""));  // Compares id to NSString * (works fine)
expect([[MyType alloc] init]).to(equal(@"")); // Compares id to NSString * (might work?)

Finally, because of the way C++ template work, the order of declaration of functions does matter. For instance, the equality matcher delegates comparison to the appropriate overload of the compare_equal function; this allows easy addition of custom comparators, as mentioned above. However, at the point where the compiler finds the equality matcher, it will only match the parameter types to comparator function overloads it has already seen. This mean you need to include your custom comparators before the declaration of the equality matcher. Since Cedar helpfully includes all matchers and comparators, you can do this in one of a few ways:

Import your custom comparators before Cedar’s SpecHelper.h at the top of your file. This should work fine, since your custom matchers and comparators need not included dependencies from Cedar.
Cedar checks a preprocessor value named CEDAR_CUSTOM_COMPARATORS. If you set this value Cedar will treat the value as a file path and #import it before the matcher library. Cedar also have CEDAR_CUSTOM_MATCHERS and CEDAR_CUSTOM_STRINGIFIERS (for providing custom output formats for your types).
For new matchers and comparators for Cocoa types (CGRect, UIView, NSIndexPath, AVPlayer, etc.), feel free to contribute back to Cedar so they get included by default.

Please send any comments, questions, suggestions, feature request, pull requests, etc. to the Cedar discussion mailing list. Unfortunately, messages to the Pivotal account on GitHub get lost in the jillions of messages and notifications we receive.

The Trouble With Expectations in Objective C

Adam Milligan

Wednesday, November 2, 2011

At Pivotal we write a lot of tests, or specs if you prefer. We TDD nearly everything we write, in every language we write in and on every platform we write for, so we actively work to improve every aspect of our testing tools. Personally, as I’ve written tests in Objective C I’ve found that the syntax of expectations has left much to be desired.

Like many people who write Objective C, I’ve spent a fair bit of time with languages like Ruby and JavaScript. When I write specs I often yearn for the simplicity of the expectation syntax in those languages. For example, some simple expectations in JavaScript, using Jasmine:

expect(composer.name).toEqual("Ludwig");
expect(composer.symphonies.count).toEqual(9);
expect(composer.symphonies).toContain("Eroica");
expect(composer.symphonies).not.toContain("Appassionata");

In comparison, the same expectations in Objective C, using OCHamcrest:

assertThat(composer.name, equalTo(@"Ludwig"));
assertThatUnsignedInt([composer.symphonies count], equalToUnsignedInt(9u));
assertThat(composer.symphonies, hasItem(@"Eroica"));
assertThat(composer.symphonies, isNot(hasItem(@"Appassionata"));

The challenge

I want three primary attributes from matchers: readability, ease of use, and extensibility.

Readability

With the exception of the unfortunate mismatch between the negation function (isNot) and the containment matcher (hasItem), I find the OCHamcrest expectations reasonably readable. The distinction between the first equality matcher (assertThat/equalTo) and the second (assertThatUnsignedInt/equalToUnsignedInt) is a bit jarring, though.

Ease of use

That distinction is much more of a problem for usability. The problem comes from the Objective C type system’s split personality: some variables refer to Objective C objects (NSNumber *, NSString *, NSArray *, UIView *), while the others refer to built-in types and structs (int, char *, CFArray, CGRect), and never the twain shall meet. A simple equality match must use the isEqual: method for the former, but the == operator, or something more esoteric like strncmp, for the latter. Sadly, a single matcher function has no way to determine the correct equality mechanism to use. Worse, the matcher function declarations must specify the types of their parameters, so expectations for any non-pointer types must have separate declarations for each type. The number of functions for any general purpose expectation explodes:

equalTo
equalToInt
equalToUnsignedInt
equalToFloat
equalToDouble
etc.

I can’t describe how many times I’ve failed to have the type of the assertThatX function for an expectation match the equalToX function. Frustration ensues.

Extensibility

Adding new OCHamcrest matchers isn’t terribly difficult, but you have to derive your matcher object from the HCBaseMatcher class, which means any library that includes custom matchers must link against the OCHamcrest library. If your spec target links against OCHamcrest and your custom matcher library, and your custom matcher library also links against OCHamcrest, suddenly you’re in dependency management hell.

What to do about it?

We’ve used OCHamcrest on a number of projects now, but the usability issues have become an increasing burden.

Peter Kim wrote a nice matcher library named Expecta which solves the type differentiation problem using the Objective C @encode function. We’ve used this on a couple projects, and like it more than OCHamcrest. The above example would look like this in Expecta:

expect(composer.name).toEqual(@"Ludwig");
expect([composer.symphonies count]).toEqual(9);
expect(composer.symphonies).toContain(@"Eroica");
expect(composer.symphonies).Not.toContain(@"Appassionata");

At the same time, we added a set of built-in matchers to Cedar, which solve the type differentiation problem with C++ templates, and which we designed to be extremely extensible. We’re using these matchers on some of our projects as well, and we’ve found they work nicely. Here’s the above example using Cedar matchers:

expect(composer.name).to(equal(@"Ludwig"));
expect([composer.symphonies count]).to(equal(9));
expect(composer.symphonies).to(contain(@"Eroica"));
expect(composer.symphonies).to_not(contain(@"Appassionata"));

You can read more about the Cedar matches, including how to use and extend them, here.

Performance tuning an old Pivotal app – how I did it

Nathan Wilmes

Wednesday, September 21, 2011

Last week, I was tasked with diving into Pivotal’s allocations application to figure out why it was operating so slowly and hopefully make it a bit better. The application was written as a side project about 4 years ago, and clearly showed its age. It’s not every day I run into an application that uses RJS! Anyways, I was able to use an incremental refactoring-based approach to improve the speed by about 80% or so. Edward and Josh Knowles suggested that I write up a bit about what I saw and how I improved it, in hopes that other engineers can make use of these performance tuning and refactoring concepts on other projects.

So, without further ado…

Set up – Measurement measurement measurement

Any time I look into a performance feature, I try to focus on getting a clear piece of functionality to optimize (in this case, the main project matrix page) and measure the heck out of its performance both on production and locally. So, the first place to go was NewRelic. NewRelic told me a couple of things – first off, there were a lot of database queries hogging most of the time. Secondly, most of those database queries involved an ActiveRecord object called ‘PersonRange’.

Adding some manual benchmarking (it’s easy! Just add a ‘stamp’ function and a before/after filter to generate a report of your timestamps) told me that many of the database queries were actually happening during view processing – a big no-no. I had a direction of investigation.

So why did the view take so long?

Like a lot of Rails projects, the view on allocations relies on a series of partials to generate a large matrix. All of these partials are looped – and with the main matrix page looping over projects, each project looping over weeks, each week looping over allocation tiles, you can imagine how the numbers add up quickly.

My first line of improvement was to streamline the innermost partial as much as possible. First off, I replaced the partial with a helper method. In general, looping over a partial is slower than calling the partial as a collection, which is in turn slower than using a helper method to generate markup. The innermost partial was very simple and easily lent itself to being a helper method. For good measure, I turned the ‘loop over allocation tiles’ code snippet into a helper method as well.

When I did this, I naturally started looking at the parameters this method needed. One strangeness – a random lookup hash named ‘roles’ was passed to this method/partial. The partial then looked up an the person’s role from this hash. The lookup hash itself was generated through a DB query generated by a helper method in the next outer partial (project_week_cell), so it generated a DB query per project per week.

On a parallel note, we also needed to look up people’s locations on a week by week basis, and there were some on the fly DB calls happening in the view layer for this as well.

So where did role and location come from? Lo and behold, both of these properties were methods of a single PersonRange object.

My direction was clear. In the controller layer, I made one set of queries to figure out the relevant person range for each person for each week. This cache was used for all location and role questions from that point forward. The cache was plopped into the main ProjectAllocationMatrix object along with the other preexisting caches, of which there were many. Boom, 50% speed improvement.

More streamlining

Before I could tackle a bigger refactor, I needed to simplify and organize the code a bit. The codebase had some obvious things to organize, that wouldn’t affect the performance much but would clarify flows and responsibilities.

There were some helper methods that didn’t have anything to do with view logic. The easy refactor for these kinds of methods is to figure out which of their arguments is the ‘important object’, and move the method into becoming an instance method of that object.
I found a few repeated patterns embedded in the views – an expression of ‘billable + unbillable + overhead’ that took three separate collections and added them together in this specific order. These three collections were only used to be added together in this way. An easy refactor consolidated them into one collection method. In the process, I simplified the calculations from three separate selects to a single sort.
One of the caches stored a person’s last unique set of initials. This cache was then postprocessed to generate an abbreviated name for that person. It seemed more useful to make this cache store the entire abbreviated name, to reduce postprocessing.
Some of the caches were exposed so that other code (mostly view logic) used them directly as a hash, knowing exactly how the cache was organized. I reduced the cohesion between the caches and the view layer, adding access methods to the matrix object that held the caches.

Taking another passthrough for performance

Even after these changes, I was still seeing some database calls in the view layer. I decided to track them down and get rid of them.

One of them was a ‘find location by id’ lookup. As it turned out, locations were an association from PersonRange, so making the person range cache join with location in the query was a no-brainer and removed all those queries.
The time calculating allocations for a week was a bit long. Looking at this code, it was running a SQL query then using Ruby to first group allocations by project ID, then sort by person for each project. I couldn’t imagine that doing an individual sort for each project was terribly efficient. I decided to do the sort first, then select the projects. This shaved a few milliseconds – no big deal.
Some of the matrix caches weren’t all that useful any more. There was a ‘locations by people’ cache that was used a bit. The person ranges cache could do everything this cache cared about and more – it also knew if a person’s location changed from week to week! I killed the other cache and converted to using the better information.
The next closest partial was ‘project_week_cell’, which was surrounded by a loop to render individual copies of this partial. I switched it to a collection partial.
I also found out about a code branch that stored abbreviated names on a person by person basis in the database rather than calculating it for each matrix. I incorporated it and removed another cache calculation from the matrix.
Around this time, I realized I could tune the ‘resource_target’ cache to cache what we actually cared about, target counts. Shifting the cache shaved a bit of time off of matrix generation in general.

All of these changes chopped another 25% or so off of the load time.

Drag/drop performance

Now the basic matrix page render was in fairly decent shape, and I moved onto the other mandate – making the drag/drop operate more smoothly. The mechanism was basically that it would perform the allocation change, recalculate the matrix for the changed projects, and then render RJS that refreshed the project rows for these changed projects.

When I looked at performance, I discovered that most of the time was spent generating two copies of the allocation matrix. The first matrix was the one mentioned in the controller (just calculating for those two projects); the second matrix was a full matrix calculated to generate refreshed billable percentages.

The interesting thing was that both matrices took almost the same time as each other. Restricting projects did not matter for performance.

As a first pass, I decided that the ‘restricted projects’ matrix was useless. If we used the bigger matrix for everything, we only had to calculate everything once and we would have everything we needed. I made this conversion and I cut server time in roughly half.

We have to refresh large chunks of HTML? Really?

In order to cut down what the server needed to refresh, I knew I needed to decouple the server-side more from the client. I’m a big fan of the Backbone-y/JSON-data-streamy approach to AJAX calls. The basic idea is this – you tell the server to do some work using AJAX, and the server returns changed model data as JSON. The server does not generate HTML markup or Javascript.

Now getting to this point is a hard problem. As an intermediate step, I decided to move the Javascript aspects of the RJS up into the client and get the server render simplified into ‘these are the chunks of HTML that I am updating’. I went with a simple JSON format using element HTML IDs as the keys and the markup as the values.

Going through this exercise was interesting. I found that many of the HTML element IDs could simply go away, as the Javascript no longer needed to refer to them to find information. I did not have Javascript model objects (that seemed like biting off too much), so I went with data attributes to store basic project/person/week IDs. Still, the markup started shrinking, which was great news.

More Javascript streamlining followed. I identified some ‘ondblclick’/'onclick’ attributes in the markup and switched over to the jQuery live event methodology to manage them more simply. The draggable elements and droppable elements became full-fledged Javascript objects (intended to eventually be Backbone views), with a matrix object to manage them.

I was now ready to do another performance pass (beyond the minor improvements from having smaller markup).

So why do we need to recalculate the whole matrix, anyways?

I returned to some of the oddities of the first drag/drop pass.

First oddity – making an allocation matrix with only 2 projects of interest was just about as expensive as making an allocation matrix with all projects. I discovered that the reason was buried in the caches – there was a low level shared cache of allocation information used both to calculate project allocations and unallocated people.

Getting rid of this cache and making these two calculations retrieve just the information they needed was the way to go. When I did this, the full matrix stayed at about the same performance level it was before. The smaller matrix, however, became much faster.

This led to the question of whether I could get away with only using the smaller matrix. The answer appeared to be ‘yes’, provided I figured out how to keep the billable percentages over all projects up to date.

Talking to Edward, I realized that the billable calculations didn’t really need a server to calculate them. All of the information they needed could be retrieved from the page – hence, the billable percentages could become a client-side concept. I switched the billable headers to be rendered using Javascript rather than the server, and then things got much lighter.

So where to, now?

Every refactor leads to a new refactoring idea. Even though my week was finished, I saw plenty of other things that could be tightened up. Among my random thoughts:

This project is using multiple Javascript frameworks – YUI, jQuery, jQuery-ui, and prototype. Prototype/YUI are older technologies and we don’t really need to use them any more if we go through a conversion pass.
YUI drag/drop requires element IDs to identify elements, and jQuery uses selectors instead. I know which technology I’d rather use. Guess which one allocations currently uses, and how many HTML IDs needed to be added to the markup because of this.
Some of the HTML classes were extremely duplicated because they were on the wrong elements. For example, I would see a billable project row with a whole bunch of allocation cells, each with the class ‘in_billable_project’. Why not give that class to the project row as a whole?
I ended up with drag/drop returning chunks of HTML markup. Chunks of basic allocation data would be a lot more lightweight, and managing cells on the client side would open up some nice possibilities for visual effects.