Nicholas Radford
2011-May-04 21:02 UTC
[Ironruby-core] Fwd: WorkItem Issue #6095 - "IronRuby 1.1.3 Can''t count"
Hey all, ? ?I thought I''d look at issue #6095 on codeplex (http://ironruby.codeplex.com/workitem/6095) as my first contribution. ? ?Having a dig through the code lead me to ArrayOps which then lead me IListOps in the IronRuby.Libraries project. ? ?I noticed IListOps.Length had the [RubyMethod("count")] attribute method which only returned the arrays count. Have to some digging (and unneeded code writing then reverting) ? ?I looked at the ruby doc and saw Array mixed in Enumerable, looked around and found that IListOps was indeed marked to include Enumerable and sure enough found the 3 count methods. ? ?One for no parameters, one with, and one with a block that did all they were supposed to do. ? ?So ?the RubyMethod declaration for count in IListOps was overriding the method declarations in Enumerable once the ruby class was put together at runtime. ? ?I removed the RubyMethod declaration on IListOps compiled and ran the IronRuby console, tried out all three versions of count and they all worked. ? ?Now, I can see why the count declaration on IListOps was put there, because accessing the internal count property on RubyArray is much quicker than counting and iterating over the collection, however, the ruby methods length and size for quick access to the number of elements in the RubyArray. ? I see two ways of fixing the bug, ? ? ?1. removing the [RubyMethod("count")] declaration from IListOps.Length which would make count inefficent on IList''s. ? ? ?2. reimplementing the object and block variations of count in IListOps In my humble opinion, 1 would be best, but since this is my first change I''d thought I''d get some input. Cheers, -- Nik Radford Mobile: 07884 254 866 Email: nikradford at googlemail.com -- Nik Radford Mobile: 07884 254 866 Email: nikradford at googlemail.com
Ryan Riley
2011-May-04 22:20 UTC
[Ironruby-core] Fwd: WorkItem Issue #6095 - "IronRuby 1.1.3 Can''t count"
On Wed, May 4, 2011 at 2:02 PM, Nicholas Radford <nik at terminaldischarge.net>wrote:> Hey all, > > I thought I''d look at issue #6095 on codeplex > (http://ironruby.codeplex.com/workitem/6095) as my first contribution. >> I see two ways of fixing the bug, > 1. removing the [RubyMethod("count")] declaration from > IListOps.Length which would make count inefficent on IList''s. > 2. reimplementing the object and block variations of count in IListOps > > In my humble opinion, 1 would be best, but since this is my first > change I''d thought I''d get some input. >I''m partial to option 2. As Ruby is already a slow language, we should try to optimize it wherever possible. In addition, 2 keeps to the trend in other .NET languages. For example, I believe LINQ takes advantage of existing methods on Lists when it can. Ryan -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20110504/2d00de52/attachment.html>
Orion Edwards
2011-May-05 02:14 UTC
[Ironruby-core] Fwd: WorkItem Issue #6095 - "IronRuby 1.1.3 Can''t count"
> > I see two ways of fixing the bug, >> >> 1. removing the [RubyMethod("count")] declaration from >> IListOps.Length which would make count inefficent on IList''s. >> 2. reimplementing the object and block variations of count in >> IListOps >> >> In my humble opinion, 1 would be best, but since this is my first >> change I''d thought I''d get some input. >> >> > I''m partial to option 2. As Ruby is already a slow language, we should try > to optimize it wherever possible. In addition, 2 keeps to the trend in other > .NET languages. For example, I believe LINQ takes advantage of existing > methods on Lists when it can. >Likewise. Because arrays are used absolutely everywhere in ruby, making a common method like count slower will have a whole lot of hidden performance penalties... Anything we can do to make these core classes faster is a win for everyone -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20110505/5c62542a/attachment.html>
Nicholas Radford
2011-May-05 08:32 UTC
[Ironruby-core] Fwd: WorkItem Issue #6095 - "IronRuby 1.1.3 Can''t count"
>> I''m partial to?option 2. As Ruby is already a slow language, we should try >> to optimize it wherever possible. In addition, 2 keeps to the trend in other >> .NET languages. For example, I believe LINQ takes advantage of existing >> methods on Lists when it can. > > Likewise. Because arrays are used absolutely everywhere in ruby, making a > common method like count slower will have a whole lot of hidden performance > penalties... Anything we can do to make these core classes faster is a win > for everyoneAlright, I''ll get onto doing option 2 when I get home this evening. Thanks guys.
Nicholas Radford
2011-May-07 07:46 UTC
[Ironruby-core] Fwd: WorkItem Issue #6095 - "IronRuby 1.1.3 Can''t count"
On 5 May 2011 09:32, Nicholas Radford <nik at terminaldischarge.net> wrote:>>> I''m partial to?option 2. As Ruby is already a slow language, we should try >>> to optimize it wherever possible. In addition, 2 keeps to the trend in other >>> .NET languages. For example, I believe LINQ takes advantage of existing >>> methods on Lists when it can. >> >> Likewise. Because arrays are used absolutely everywhere in ruby, making a >> common method like count slower will have a whole lot of hidden performance >> penalties... Anything we can do to make these core classes faster is a win >> for everyone > > Alright, I''ll get onto doing option 2 when I get home this evening. > > Thanks guys. >Quick question guys, when pushing this to my repo on github, should I push to a topic branch or push to master? -- Nik Radford Mobile: 07884 254 866 Email: nikradford at googlemail.com
Nicholas Radford
2011-May-07 08:12 UTC
[Ironruby-core] Fwd: WorkItem Issue #6095 - "IronRuby 1.1.3 Can''t count"
> > Quick question guys, when pushing this to my repo on github, should I > push to a topic branch or push to master? >Well I pushed it to a topic branch, just in case you guys required changes :) As the contributing documentation says, would someone mind giving me a code review? Code can be found at https://github.com/sekhat/main/tree/bugfix/issue-6095 Cheers.
Tomas Matousek
2011-May-08 01:17 UTC
[Ironruby-core] Fwd: WorkItem Issue #6095 - "IronRuby 1.1.3 Can''t count"
Yes, this is the right process - fix it in a branch and send a pull request. The change looks great! Merged into main and closed the bug. Thanks, Tomas -----Original Message----- From: ironruby-core-bounces at rubyforge.org [mailto:ironruby-core-bounces at rubyforge.org] On Behalf Of Nicholas Radford Sent: Saturday, May 07, 2011 1:13 AM To: ironruby-core at rubyforge.org Subject: Re: [Ironruby-core] Fwd: WorkItem Issue #6095 - "IronRuby 1.1.3 Can''t count"> > Quick question guys, when pushing this to my repo on github, should I > push to a topic branch or push to master? >Well I pushed it to a topic branch, just in case you guys required changes :) As the contributing documentation says, would someone mind giving me a code review? Code can be found at https://github.com/sekhat/main/tree/bugfix/issue-6095 Cheers. _______________________________________________ Ironruby-core mailing list Ironruby-core at rubyforge.org http://rubyforge.org/mailman/listinfo/ironruby-core