This repository has been archived by the owner on Apr 17, 2018. It is now read-only.
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Update Benchmarking method definitions to use class_eval
* This works-around some problems with JRuby 1.4 not being able to handle a block with the arguments that were defined.
- Loading branch information
fe94d1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanx, that will run with jruby :-)
fe94d1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw, I have a patch in the active_support branch of dm-core (that goes with dm-rails) that removes use of objectspace to be jruby friendly: http://github.com/sundbp/dm-core/commit/9dbbd75b0d949341cd912cf106b6709753c95208
Seems like a good idea to incorporate?
fe94d1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dkubb
ObjectSpace has a significant overhead in jruby: http://kenai.com/projects/jruby/pages/PerformanceTuning#Disabling_ObjectSpace
I do not understand the consequences of sundbp patch but I agree with him the remove the use of ObjectSpace
fe94d1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll admit to being a bit ignorant of the exact impact of the change I made. I've put it in my fork and used it for my use cases of the last month and it seems to work well - can't notice any visible difference on MRI, but no real investigation done.
fe94d1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that I would really like a way to remove the usage of ObjectSpace from that code, but I'm not entirely sure what the impact will be.
What that code does (or so I understand it), is that it allows you to match against the model object without holding a reference to it, preventing circular dependencies, which can cause problems with objects not being GC'd.
Although in this case I don't understand the point of testing self.class against the model. I mean, the string that this creates eventually gets passed down to an instance method, and inlined inside a single method. Since it's being executed within an instance method, by definition, doesn't that mean that self.class would always be equal to or a descendant of the class that defined the hook?
If someone else could validate what I say above to be true, I would gladly apply sundbp's patch.
fe94d1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll try to spend some time over the weekend on understanding the code better and comment.
For the record I'm not sure if I came up with that patch or if I incorporated it from elsewhere, memory a bit fuzzy.
fe94d1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really would appreciate this, since having ObjectSpace with jruby when it is disabled feels not right and enabling it feels even more wrong. thanx in advance.
fe94d1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that the google app engine guys do something to replace that method in the rails_dm_datastore gem with something equivalent, which might be worth looking into.
fe94d1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, cool. Found extlib.rb and local_object_space.rb here quite interesting and possibly better solution:
http://github.com/joshsmoore/rails_dm_datastore/tree/master/lib/rails_dm_datastore/
fe94d1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to me we could adopt the solution the rails_dm_datastore people have put together in dm-core (since moved from extlib) without changing functionality or type of implementation and it'll work for both MRI and jruby (seems rubinius implemented some "missing" objectspace stuff last month but still good idea to avoid it if possible).
Want me to put together a patch vs dm-core (active_support branch) using this sort of implementation?
fe94d1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, please - you are the one who has the best background now and I would really appreciate dm-core to be one step more jruby friendly :-)
fe94d1a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've put it together in: http://github.com/sundbp/dm-core/commits/next/
Sent a pull request.