Project

General

Profile

Bug #14908

Enumerator::Lazy creates unnecessary Array objects.

Added by chopraanmol1 (Anmol Chopra) 11 months ago. Updated 8 months ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
ruby -v:
ruby 2.6.0dev (2018-07-11 trunk 63949) [x86_64-linux]
[ruby-core:87907]

Description

Benchmark result on trunk:

                 user     system      total        real
Lazy:        0.120000   0.000000   0.120000 (  0.119958)
Normal:      0.056000   0.004000   0.060000 (  0.062848)
             2.142857   0.000000        NaN (  1.908698)
++++++++++++++++++++++++++++++++++++++++++++++++++
Lazy:
Total allocated: 122240 bytes (3033 objects)
Total retained:  0 bytes (0 objects)

allocated memory by class
--------------------------------------------------
    120480  Array
       880  Proc
       384  Enumerator::Lazy
       264  Object
       168  Enumerator::Generator
        64  Enumerator::Yielder

allocated objects by class
--------------------------------------------------
      3012  Array
        11  Proc
         3  Enumerator::Generator
         3  Enumerator::Lazy
         3  Object
         1  Enumerator::Yielder
++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++
Normal:
Total allocated: 72120 bytes (3 objects)
Total retained:  0 bytes (0 objects)

allocated memory by class
--------------------------------------------------
     72120  Array

allocated objects by class
--------------------------------------------------
         3  Array
++++++++++++++++++++++++++++++++++++++++++++++++++

As you may observe an extra array is created for every final element. Enumerator::Yielder#yield method has arity of -1 which wrap every elements in array. The same goes for Enumerator::Yielder#<< method, I'm proposing to change arity for Enumerator::Yielder#<< to 1 from -1 and use it internally for the lazy enum. It will also make << method definition consistent with other classes(Array, String & etc).

I've applied the following set of changes:

https://github.com/ruby/ruby/pull/1912

which result in the following:

                 user     system      total        real
Lazy:        0.108000   0.000000   0.108000 (  0.108484)
Normal:      0.052000   0.008000   0.060000 (  0.062528)
             2.076923   0.000000        NaN (  1.734961)
++++++++++++++++++++++++++++++++++++++++++++++++++
Lazy:
Total allocated: 2240 bytes (33 objects)
Total retained:  0 bytes (0 objects)

allocated memory by class
--------------------------------------------------
       880  Proc
       480  Array
       384  Enumerator::Lazy
       264  Object
       168  Enumerator::Generator
        64  Enumerator::Yielder

allocated objects by class
--------------------------------------------------
        12  Array
        11  Proc
         3  Enumerator::Generator
         3  Enumerator::Lazy
         3  Object
         1  Enumerator::Yielder
++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++
Normal:
Total allocated: 72120 bytes (3 objects)
Total retained:  0 bytes (0 objects)

allocated memory by class
--------------------------------------------------
     72120  Array

allocated objects by class
--------------------------------------------------
         3  Array
++++++++++++++++++++++++++++++++++++++++++++++++++

This changes reduces the memory utilization and also by a tiny fraction improves performance for the lazy enumerator


Files

lazy_test.rb (1.26 KB) lazy_test.rb Benchmark File chopraanmol1 (Anmol Chopra), 07/11/2018 01:09 PM

Associated revisions

Revision aefdd4e2
Added by nobu (Nobuyoshi Nakada) 8 months ago

Lazy Enumerator reduce intermediate array creation

[ruby-core:87907] [Bug #14908] [Fix GH-1912]

From: Anmol Chopra chopraanmol1@gmail.com

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@64770 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 64770
Added by nobu (Nobuyoshi Nakada) 8 months ago

Lazy Enumerator reduce intermediate array creation

[ruby-core:87907] [Bug #14908] [Fix GH-1912]

From: Anmol Chopra chopraanmol1@gmail.com

Revision 64770
Added by nobu (Nobuyoshi Nakada) 8 months ago

Lazy Enumerator reduce intermediate array creation

[ruby-core:87907] [Bug #14908] [Fix GH-1912]

From: Anmol Chopra chopraanmol1@gmail.com

History

#1

Updated by chopraanmol1 (Anmol Chopra) 11 months ago

  • Description updated (diff)
#2

Updated by chopraanmol1 (Anmol Chopra) 11 months ago

  • Description updated (diff)

Updated by shyouhei (Shyouhei Urabe) 11 months ago

Understand the problem. The proposed fix however involves spec change. We need to discuss effects of it before applying this patch.

Updated by chopraanmol1 (Anmol Chopra) 11 months ago

shyouhei (Shyouhei Urabe) wrote:

Understand the problem. The proposed fix however involves spec change. We need to discuss effects of it before applying this patch.

If this is a big breaking change than alternate will be to create a different method for Enumerator::Lazy's internal use. I'm also up for updating the patch to reflect that. But I think for future release it will make more sense to have Enumerator::Yielder#<< to have an arity of 1 if you consider syntax use case.

But for the backporting purpose, I'm more inclined to create a new method.

Updated by Eregon (Benoit Daloze) 11 months ago

Changing Enumerator::Yielder#<< to have arity 1 seems fine to me, as I guess nobody calls << on an Enumerator::Yielder with more than 1 argument, isn't it?

Updated by chopraanmol1 (Anmol Chopra) 11 months ago

Eregon (Benoit Daloze) wrote:

Changing Enumerator::Yielder#<< to have arity 1 seems fine to me, as I guess nobody calls << on an Enumerator::Yielder with more than 1 argument, isn't it?

Yes, that will be the general case. Exception:

.send(:<<,...)
.<<(...)

Updated by marcandre (Marc-Andre Lafortune) 11 months ago

Indeed, as long as Yielder#yield is kept with arity -1 (as in this patch), indeed I don't think that would be an "incompatibility" we should worry about.

#8

Updated by chopraanmol1 (Anmol Chopra) 11 months ago

  • Description updated (diff)
#9

Updated by chopraanmol1 (Anmol Chopra) 9 months ago

  • Description updated (diff)

Updated by knu (Akinori MUSHA) 8 months ago

I'm the original author of Enumerator & Yielder and I don't think I particularly intended to make << accept many arguments. << was an alias of yield, and I just didn't bother to make a separate function. So, I'd say go ahead.

Updated by chopraanmol1 (Anmol Chopra) 8 months ago

All the responses for the proposed patch are positive till date. shyouhei (Shyouhei Urabe) can we go ahead with this now?

Updated by shyouhei (Shyouhei Urabe) 8 months ago

No objection. I guess nobu (Nobuyoshi Nakada) is trying to fix it.

#13

Updated by nobu (Nobuyoshi Nakada) 8 months ago

  • Status changed from Open to Closed

Applied in changeset trunk|r64770.


Lazy Enumerator reduce intermediate array creation

[ruby-core:87907] [Bug #14908] [Fix GH-1912]

From: Anmol Chopra chopraanmol1@gmail.com

Also available in: Atom PDF