search

solnic / virtus

[DISCONTINUED ] Attributes on Steroids for Plain Old Ruby Objects
MIT License
3.77k stars 229 forks source link

prevent coercer from throwing unnecessary exceptions #359

Open skippy opened 8 years ago

skippy commented 8 years ago

I was benchmarking my app, and the number 2 item on the list, for the # of objects created, was from this line: https://github.com/solnic/virtus/blob/master/lib/virtus/attribute/coercer.rb#L33

     TOTAL    (pct)     SAMPLES    (pct)     FRAME
    464008  (43.2%)      256008  (23.8%)     Time.parse
    175000  (16.3%)      175000  (16.3%)     Coercible::Coercer::Object#raise_unsupported_coercion

this patch removes this from the top 20 object-creation offenders, courtesy of the stackprof output.

It is a similar story for CPU profiling:

     TOTAL    (pct)     SAMPLES    (pct)     FRAME
       152  (24.8%)          99  (16.2%)     Time.parse
       443  (72.3%)          90  (14.7%)     Virtus::Attribute::Coercer#call
        68  (11.1%)          68  (11.1%)     Coercible::Coercer#[]
        29   (4.7%)          29   (4.7%)     Coercible::Coercer::Object#raise_unsupported_coercion

after this patch, the Coercible::Coercer::Object#raise_unsupported_coercion no longer shows up, for a rough savings of 4.7%, but then the number of calls to Virtus::Attribute::Coercer#call and Coercible::Coercer#[] is also slashed.

     TOTAL    (pct)     SAMPLES    (pct)     FRAME
       151  (26.7%)         103  (18.2%)     Time.parse
        36   (6.4%)          36   (6.4%)     Coercible::Coercer#[]
       222  (39.2%)          27   (4.8%)     Virtus::Attribute::Coercer#call
       240  (42.4%)          25   (4.4%)     Virtus::AttributeSet#each

For my specific code, I also noticed a 22% performance improvement from this one change (granted, the app I'm using this in is in a tight loop, but still... wow!)

The usual caveat of microbenchmarks applies, plus I'm using stackprof, which does sampling, so every result is slightly different. But I think you'll agree this is a simple change with a lot of upside. I wish all performance patches were this 'easy'

skippy commented 8 years ago

I originally changed this in lib/virtus/attribute/coercer.rb but that failed as I forgot about the boolean edge-case. I moved it to check just for strings. This is still quite good for my edge case, but it may not be as applicable a performance boost for everyone. Let me know if you want me to put it back into lib/virtus/attribute/coercer.rb and revisit the tests