Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typecast on writing #151

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

aderyabin
Copy link

Currently, typecasting works on attribute reading. There are two defects:
– every reading is every typecasting.
– if typecasted class has attributes they can't be changed, because typecasting creates a new instance every time.

I suggest typecasting on writing. It avoids from both defects

@film42
Copy link

film42 commented Feb 8, 2016

@aderyabin I think we should actually find a way to move away from creating a new instance of a type caster class because I've found it to add an enormous amount of overhead in object allocation. I started moving over to setting a :typecaster directly, because I found an order of magnitude in speed-up by not creating a new typecaster each time.

EDIT: I should have mentioned, I saw this performance bottleneck while running under a profiler (which is very costly in terms of allocations), but in real world MRI examples, it's about 10% faster (2 second reduction after 10k iterations of a model with 93 attributes).

@aderyabin
Copy link
Author

@film42 Could you please share benchmark script?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants