Re: [Rails-core] Increase performance

[Xavier Noria]

On Sat, May 7, 2016 at 8:19 PM, Andrey Molchanov  wrote:

Xavier, do you want find another similar place for changes?
>

We generally do not accept cosmetic changes in GitHub (unless they are part
of a regular patch) because there is too much going on in the issue
tracker, but if you find one or two more cases worth having a look please
do propose them and /cc me.

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

Re: [Rails-core] Increase performance

[Andrey Molchanov]

Xavier, do you want find another similar place for changes?

суббота, 7 мая 2016 г., 20:47:59 UTC+3 пользователь Andrey Molchanov 
написал:
>
> Oh, of course use until is better
> Yes, I would create PR.
>
> суббота, 7 мая 2016 г., 20:44:30 UTC+3 пользователь Xavier Noria написал:
>>
>> Agree, this alternative reads better, and both options return nil.
>>
>> Alternatively you could use until and leave the condition positive:
>>
>> until node.equal? NULL
>>   yield node
>>   node = node.parent
>> end
>>
>> Would you like to volunteer a PR? (Please /cc @fxn if you do.)
>>
>

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

Re: [Rails-core] Increase performance

[Andrey Molchanov]

Oh, of course use until is better
Yes, I would create PR.

суббота, 7 мая 2016 г., 20:44:30 UTC+3 пользователь Xavier Noria написал:
>
> Agree, this alternative reads better, and both options return nil.
>
> Alternatively you could use until and leave the condition positive:
>
> until node.equal? NULL
>   yield node
>   node = node.parent
> end
>
> Would you like to volunteer a PR? (Please /cc @fxn if you do.)
>

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

Re: [Rails-core] Increase performance

[Andrey Molchanov]

Guys, thanks you!
And to close subject the last question. What do you think about this 
replace:

loop do
  break if node.equal? NULL
  yield node
  node = node.parent
end

to

while !node.equal? NULL
  yield node
  node = node.parent
end

In this case readability and speed are higher. 


суббота, 7 мая 2016 г., 10:25:08 UTC+3 пользователь Xavier Noria написал:
>
> Exactly.
>
> On Saturday, 7 May 2016, Allen Madsen  
> wrote:
>
>> I believe what Xavier means is that you need to demonstrate at the
>> places where you would do the replacement that the speed up is worth
>> the readability cost. If on a single request, that code executes once,
>> it's probably not noticeable. That's why in the benchmark you need to
>> run it 100 million times. If you can demonstrate that the place
>> containing the `loop` is a hotspot that gets executed many times in a
>> request, then the optimization is probably justified. That's why it
>> wouldn't just be a find and replace for all instances.
>> Allen Madsen
>> http://www.allenmadsen.com
>>
>>
>> On Fri, May 6, 2016 at 7:47 AM, Andrey Molchanov <> wrote:
>> > Thanks for your feedback.
>> > I agree with you about use idioms for better readable code. Its cool, 
>> but
>> > Rails has many places where this not use. A lot of code can be corrected
>> > according to this, but this is not done.
>> > And I thought, why not use it in favor of speed?
>> > That is what I was based when wrote this message.
>> >
>> >
>> > пятница, 6 мая 2016 г., 14:31:40 UTC+3 пользователь Xavier Noria 
>> написал:
>> >>
>> >> In general, the Rails code base wants to use Ruby idiomatically.
>> >>
>> >> loop is the most succinct idiom in Ruby for those kinds of loops, see 
>> for
>> >> example:
>> >>
>> >>
>> >> 
>> https://github.com/rails/rails/blob/254f57ca3668398a5fcfd4f63be5d91c4c3b1cd4/actioncable/lib/action_cable/connection/stream_event_loop.rb#L66
>> >>
>> >> If a Ruby programmer sees a while true there instead, generally 
>> speaking
>> >> they would shake their heads a little bit. Why is this not a loop? A 
>> comment
>> >> would be needed: "This while true is here for performance".
>> >>
>> >> When is it OK to do a little strange thing for performance? Where it
>> >> matters, not systematically across the code base. So, for example, if 
>> you
>> >> change loop with while true in the previous example, probably there 
>> won't be
>> >> any noticeable difference. So you just don't.
>> >>
>> >> And if the gain is tiny, the cost of writing something less idiomatic,
>> >> elegant, or readable is still not worth it. Because code has to be 
>> read.
>> >>
>> >> You depart from this with a scalpel, precisely where it pays off.
>> >>
>> > --
>> > You received this message because you are subscribed to the Google 
>> Groups
>> > "Ruby on Rails: Core" group.
>> > To unsubscribe from this group and stop receiving emails from it, send 
>> an
>> > email to rubyonrails-core+unsubscr...@googlegroups.com.
>> > To post to this group, send email to rubyonrails-core@googlegroups.com.
>> > Visit this group at https://groups.google.com/group/rubyonrails-core.
>> > For more options, visit https://groups.google.com/d/optout.
>>
>> --
>> You received this message because you are subscribed to the Google Groups 
>> "Ruby on Rails: Core" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to rubyonrails-core+unsubscr...@googlegroups.com.
>> To post to this group, send email to rubyonrails-core@googlegroups.com.
>> Visit this group at https://groups.google.com/group/rubyonrails-core.
>> For more options, visit https://groups.google.com/d/optout.
>>
>
>
> -- 
> Sent from Gmail Mobile
>

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

Re: [Rails-core] Increase performance

[Xavier Noria]

Exactly.

On Saturday, 7 May 2016, Allen Madsen  wrote:

> I believe what Xavier means is that you need to demonstrate at the
> places where you would do the replacement that the speed up is worth
> the readability cost. If on a single request, that code executes once,
> it's probably not noticeable. That's why in the benchmark you need to
> run it 100 million times. If you can demonstrate that the place
> containing the `loop` is a hotspot that gets executed many times in a
> request, then the optimization is probably justified. That's why it
> wouldn't just be a find and replace for all instances.
> Allen Madsen
> http://www.allenmadsen.com
>
>
> On Fri, May 6, 2016 at 7:47 AM, Andrey Molchanov  > wrote:
> > Thanks for your feedback.
> > I agree with you about use idioms for better readable code. Its cool, but
> > Rails has many places where this not use. A lot of code can be corrected
> > according to this, but this is not done.
> > And I thought, why not use it in favor of speed?
> > That is what I was based when wrote this message.
> >
> >
> > пятница, 6 мая 2016 г., 14:31:40 UTC+3 пользователь Xavier Noria написал:
> >>
> >> In general, the Rails code base wants to use Ruby idiomatically.
> >>
> >> loop is the most succinct idiom in Ruby for those kinds of loops, see
> for
> >> example:
> >>
> >>
> >>
> https://github.com/rails/rails/blob/254f57ca3668398a5fcfd4f63be5d91c4c3b1cd4/actioncable/lib/action_cable/connection/stream_event_loop.rb#L66
> >>
> >> If a Ruby programmer sees a while true there instead, generally speaking
> >> they would shake their heads a little bit. Why is this not a loop? A
> comment
> >> would be needed: "This while true is here for performance".
> >>
> >> When is it OK to do a little strange thing for performance? Where it
> >> matters, not systematically across the code base. So, for example, if
> you
> >> change loop with while true in the previous example, probably there
> won't be
> >> any noticeable difference. So you just don't.
> >>
> >> And if the gain is tiny, the cost of writing something less idiomatic,
> >> elegant, or readable is still not worth it. Because code has to be read.
> >>
> >> You depart from this with a scalpel, precisely where it pays off.
> >>
> > --
> > You received this message because you are subscribed to the Google Groups
> > "Ruby on Rails: Core" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> > email to rubyonrails-core+unsubscr...@googlegroups.com .
> > To post to this group, send email to rubyonrails-core@googlegroups.com
> .
> > Visit this group at https://groups.google.com/group/rubyonrails-core.
> > For more options, visit https://groups.google.com/d/optout.
>
> --
> You received this message because you are subscribed to the Google Groups
> "Ruby on Rails: Core" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to rubyonrails-core+unsubscr...@googlegroups.com .
> To post to this group, send email to rubyonrails-core@googlegroups.com
> .
> Visit this group at https://groups.google.com/group/rubyonrails-core.
> For more options, visit https://groups.google.com/d/optout.
>


-- 
Sent from Gmail Mobile

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

Re: [Rails-core] Increase performance

[Allen Madsen]

I believe what Xavier means is that you need to demonstrate at the
places where you would do the replacement that the speed up is worth
the readability cost. If on a single request, that code executes once,
it's probably not noticeable. That's why in the benchmark you need to
run it 100 million times. If you can demonstrate that the place
containing the `loop` is a hotspot that gets executed many times in a
request, then the optimization is probably justified. That's why it
wouldn't just be a find and replace for all instances.
Allen Madsen
http://www.allenmadsen.com


On Fri, May 6, 2016 at 7:47 AM, Andrey Molchanov  wrote:
> Thanks for your feedback.
> I agree with you about use idioms for better readable code. Its cool, but
> Rails has many places where this not use. A lot of code can be corrected
> according to this, but this is not done.
> And I thought, why not use it in favor of speed?
> That is what I was based when wrote this message.
>
>
> пятница, 6 мая 2016 г., 14:31:40 UTC+3 пользователь Xavier Noria написал:
>>
>> In general, the Rails code base wants to use Ruby idiomatically.
>>
>> loop is the most succinct idiom in Ruby for those kinds of loops, see for
>> example:
>>
>>
>> https://github.com/rails/rails/blob/254f57ca3668398a5fcfd4f63be5d91c4c3b1cd4/actioncable/lib/action_cable/connection/stream_event_loop.rb#L66
>>
>> If a Ruby programmer sees a while true there instead, generally speaking
>> they would shake their heads a little bit. Why is this not a loop? A comment
>> would be needed: "This while true is here for performance".
>>
>> When is it OK to do a little strange thing for performance? Where it
>> matters, not systematically across the code base. So, for example, if you
>> change loop with while true in the previous example, probably there won't be
>> any noticeable difference. So you just don't.
>>
>> And if the gain is tiny, the cost of writing something less idiomatic,
>> elegant, or readable is still not worth it. Because code has to be read.
>>
>> You depart from this with a scalpel, precisely where it pays off.
>>
> --
> You received this message because you are subscribed to the Google Groups
> "Ruby on Rails: Core" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to rubyonrails-core+unsubscr...@googlegroups.com.
> To post to this group, send email to rubyonrails-core@googlegroups.com.
> Visit this group at https://groups.google.com/group/rubyonrails-core.
> For more options, visit https://groups.google.com/d/optout.

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

Re: [Rails-core] Increase performance

[Andrey Molchanov]

Thanks for your feedback.
I agree with you about use idioms for better readable code. Its cool, but 
Rails has many places where this not use. A lot of code can be corrected 
according to this, but this is not done.
And I thought, why not use it in favor of speed?
That is what I was based when wrote this message.


пятница, 6 мая 2016 г., 14:31:40 UTC+3 пользователь Xavier Noria написал:
>
> In general, the Rails code base wants to use Ruby idiomatically.
>
> loop is the most succinct idiom in Ruby for those kinds of loops, see for 
> example:
>
> https://github
> .com/rails/rails/blob/254f57ca3668398a5fcfd4f63be5d91c4c3b1cd4/actioncable
> /lib/action_cable/connection/stream_event_loop.rb#L66
>
> If a Ruby programmer sees a while true there instead, generally speaking 
> they would shake their heads a little bit. Why is this not a loop? A 
> comment would be needed: "This while true is here for performance".
>
> When is it OK to do a little strange thing for performance? Where it 
> matters, not systematically across the code base. So, for example, if you 
> change loop with while true in the previous example, probably there won't 
> be any noticeable difference. So you just don't.
>
> And if the gain is tiny, the cost of writing something less idiomatic, 
> elegant, or readable is still not worth it. Because code has to be read.
>
> You depart from this with a scalpel, precisely where it pays off.
>
>

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

Re: [Rails-core] Increase performance

[Xavier Noria]

In general, the Rails code base wants to use Ruby idiomatically.

loop is the most succinct idiom in Ruby for those kinds of loops, see for
example:

https://github
.com/rails/rails/blob/254f57ca3668398a5fcfd4f63be5d91c4c3b1cd4/actioncable
/lib/action_cable/connection/stream_event_loop.rb#L66

If a Ruby programmer sees a while true there instead, generally speaking
they would shake their heads a little bit. Why is this not a loop? A
comment would be needed: "This while true is here for performance".

When is it OK to do a little strange thing for performance? Where it
matters, not systematically across the code base. So, for example, if you
change loop with while true in the previous example, probably there won't
be any noticeable difference. So you just don't.

And if the gain is tiny, the cost of writing something less idiomatic,
elegant, or readable is still not worth it. Because code has to be read.

You depart from this with a scalpel, precisely where it pays off.

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

[Rails-core] Increase performance

[Andrey Molchanov]

I propose replace all the endless loop do to while true:

NUMBER = 100_000_000

def old_slow
  index = 0
  loop do
break if index > NUMBER
index += 1
  end
end

def new_fast
  index = 0
  while true
break if index > NUMBER
index += 1
  end
end


Benchmark.ips do |x|
  x.report("new_fast") { new_fast }
  x.report("old_slow") { old_slow }
  x.compare!
end

Calculating -
new_fast 1.000  i/100ms
old_slow 1.000  i/100ms
-
new_fast  0.655  (± 0.0%) i/s -  4.000  in   6.106470s
old_slow  0.259  (± 0.0%) i/s -  2.000  in   7.707258s

Comparison:
new_fast:0.7 i/s
old_slow:0.3 i/s - 2.52x slower


What do you think about it?

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.