Refactoring of attributes/typecasting behavior #641

Closed
lighthouse-import opened this Issue May 16, 2011 · 17 comments

Comments

Projects
None yet
2 participants

Imported from Lighthouse. Original ticket at: http://rails.lighthouseapp.com/projects/8994/tickets/3348
Created by Eric Chapweske - 2010-10-25 02:23:39 UTC

Decouples the attribute/typecasting implementation, introducing an Attributes:Store and ActiveRecord::Types. I think this simplifies things and lays the groundwork for other improvements. Assuming this seems like a good idea, I'd like to continue by improving the Types implementation and seeing if it makes sense to extract any other attribute/typecasting behavior.

Imported from Lighthouse.
Comment by Joshua Peek - 2009-10-16 00:37:35 UTC

This is pretty awesome!

But its not applying cleanly for me. Can you please rebase it.

I would love to eventually pull this stuff back into ActiveModel.

Imported from Lighthouse.
Comment by Eric Chapweske - 2009-10-17 00:55:45 UTC

Woops, this new one should apply.

Imported from Lighthouse.
Comment by Eric Chapweske - 2009-10-17 01:21:35 UTC

Sorry, I created that last patch without the tests. This is the complete version.

Imported from Lighthouse.
Comment by Repository - 2009-10-17 17:37:31 UTC

(from [f936a1f]) Refactoring attributes/types [#3348 state:resolved]

http://github.com/rails/rails/commit/f936a1f100e75082081e782e5cceb272885c2df7

Imported from Lighthouse.
Comment by Jeremy Kemper - 2010-01-21 21:45:45 UTC

This is a great refactoring but it's a huge performance regression: https://gist.github.com/3b5884a2c87755f4015d

Imported from Lighthouse.
Comment by Repository - 2010-01-21 21:52:41 UTC

(from [6d30002]) Revert "Refactoring attributes/types" [#3348 state:open]

This reverts commit f936a1f.

Conflicts:

activerecord/lib/active_record.rb
activerecord/lib/active_record/base.rb

Revert "Fixed: #without_typecast should only disable typecasting on the duplicated attributes" [#3387 state:open]

This reverts commit 2831996.

Reason :

It's not generating attribute methods properly, making object.column 5x slower.
http://github.com/rails/rails/commit/6d30002a52133bd105adb29084f4cc72b1ee847f

Imported from Lighthouse.
Comment by Eric Chapweske - 2010-01-25 17:20:55 UTC

Hey guys,

What's the performance goal for this?

Current results: reading attributes is now 3x slower, though a couple methods are a fair bit faster. (query_attribute is 6x faster). The remaining performance differences are due to the longer method call path.

I'd like to explore pulling the @attributes_cache into AR::Attributes and cache most methods by default, performance is then the same or better than 2.3.

Cheers,
-Eric

Imported from Lighthouse.
Comment by Jeremy Kemper - 2010-01-25 17:43:05 UTC

Eric, the goal is 2.3 speed or better.

Imported from Lighthouse.
Comment by Rohit Arondekar - 2010-06-17 06:36:50 UTC

Any updates to this ticket? Eric, have you had the chance to work on a new patch?

Imported from Lighthouse.
Comment by Jeremy Kemper - 2010-08-30 03:10:31 UTC

[bulk edit]

Imported from Lighthouse.
Comment by Jeremy Kemper - 2010-10-15 22:01:36 UTC

[bulk edit]

Imported from Lighthouse.
Comment by Ryan Bigg - 2010-10-19 07:33:43 UTC

Automatic cleanup of spam.

Imported from Lighthouse.
Comment by Santiago Pastorino - 2010-11-15 21:04:50 UTC

[bulk edit]

Imported from Lighthouse.
Comment by Santiago Pastorino - 2011-02-12 21:58:20 UTC

[bulk edit]

Imported from Lighthouse.
Comment by Santiago Pastorino - 2011-02-27 03:15:38 UTC

[bulk edit]

Attachments saved to Gist: http://gist.github.com/971645

Contributor

codesnik commented Mar 14, 2012

I wonder, are there any plans to refactor typecasting to active_model again?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment