MSSQL: Throwing exceptions in the hot path results in slow code

[mjc]

On MSSQL, the following code gets 2800 inserts to the CSV per second on MRI,
and 300/s (indy off) on 1.7.12.

class Widget < ActiveRecord::Base
end

CSV.open('test.csv','wb') do |csv|
  Widget.find_each do |w|
    csv << w.attributes.values
  end
end

I ran it through the JRuby profiler.
79%self was being spent in ArJdbc::MSSQL ArJdbc::MSSQL::Column.type_cast.

Since generating backtraces etc for exceptions are very slow on JRuby,
I guessed at the kind of exceptions this code would encounter.
I'm almost certain there are things I am missing, but this code now performs identically
on MRI and JRuby 1.7.12 (indy off.)

If there is anything else I need to do to make this suitable for merging, I'm happy to do it.

[kares]

Thanks Michael, it's fine to go in as is ... will maybe try to run the tests (there's currently hangs on AR 4.x with some of the transaction isolation related tests) to see if there's any regression but I'm guessing not. Polishing such as the one you did is very needed esp. for adapters such as sqlserver, thanks again!

[mjc]

@headius and @nirvdrum pointed out some other improvements to be made to this, mysql and possibly to the pg adapter (combining regexps at the very least.)

Pg adapter's column type code is about 2% of overall CPU time in one of my hot paths, so I will definitely be poking at that.

I'd be happy to poke at those, do you have any ideas about areas for perf improvement I might consider in other adapters, or other parts of these adapters?

Additionally I'm curious about how well tested you think these things are. Should I put some time into making sure they are tested enough that I won't break things that the tests won't catch?

[mjc]

Looks like this code needs some work still, don't merge it quite yet.

[mjc]

In the old code it doesn't look like the rescue value ? 1 : 0 is reachable, since pretty much everything responds to to_s. Am I missing something?

[kares]

@mjc great - I do have a few optimizations already in my pocket (currently only tested with MySQL/PG). they're mostly concerning avoiding "unnecessary" intermediate objects while getting data from JDBC (e.g. extracting bytes instead of Java strings to get a Ruby string) but those won't made it to 1.3 (due API changes would require a new "major" release) ... anything you do on the Ruby side should be good (and very valuable) to go. just run our tests if there's any regression (MySQL/PG and similar should be green - travis will tell)

if you're really in doubt, it's possible to run the rails test suite as well (it's far from green but one can check if there are additional failures with a change).

maybe, would be best if you'd opened another PR for the other stuff you're planning if it's not MS-SQL related.

as for the rescue value ? 1 : 0 part - seems not reachable ... probably an attempt to handle boolean emulation (I'm really not sure if it's needed if the tests run the same than likely not) ...

[kares]

put these on master, although I partially reverted the second one (due an introduced test failure) at c04ea07
also please try to be consistent if you're planning more work with the coding style ... MS-SQL had been hacked by a lot here and there and it was quite bad to read ... I reverted the case to a one liner, hopefully using is_a? instead of === is not a performance bottleneck :) I generally actually try to avoid it for simple cases such as a single check, let me know if I'm wrong.

the suite is green on AR 3.2 on 4.x there are some (mostly date related) regressions which I have not looked into ... at least I've avoided the dead-lock for now e5f3cd8 (just in case you're planning any more work - it's best to run the tests)

[mjc]

Ah, sorry about the regression. I should be a bit more careful next time.