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

Added register_block_for method to register Railties engines blocks(eg: rake_tasks, generators) #26052

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@fidelisrafael
Contributor

fidelisrafael commented Aug 4, 2016

Summary

This is only a small improvement I think is valid, I mean, this code can be kept how is today...but this change probably makes code more simple and readable, and my main goal submiting this PR is to create an API method to register blocks, since the each_registered_block is used in code with the similar purpose.

Other Information

I started this using instance_eval, but I know this can't be good, so I benchmarked 4 ways of doing this same thing:

  • instance_eval (the slowest, just for reference)
  • pass initialized variable as reference (first implementation for this PR)
  • symbol as identifier/instance variable get/set (current implementation in this PR)
  • current railties implementation

Benchmark code and results are here: https://gist.github.com/fidelisrafael/d8509eea8ddac3e61c4ada2d706ee53b

Benchmark results

Please, refer to this gist to analisys the code behing this benchmark results:

simple benchmark

==================================================
single `rake_task` method call inside loop
==================================================

                                                         user     system      total        real
instance_eval                                        2.350000   0.020000   2.370000 (  2.366487)
variable init                                        0.180000   0.000000   0.180000 (  0.176445)
instance variable get/set                            0.160000   0.000000   0.160000 (  0.162669)
current railties                                     0.250000   0.000000   0.250000 (  0.252082)

==================================================
Single `generators` method call inside loop
==================================================

                                                         user     system      total        real
instance_eval                                        2.260000   0.020000   2.280000 (  2.273899)
variable init                                        0.120000   0.000000   0.120000 (  0.115172)
instance variable get/set                            0.160000   0.000000   0.160000 (  0.161243)
current railties                                     0.160000   0.000000   0.160000 (  0.160353)

==================================================
4 methods calls inside each loop
==================================================

                                                         user     system      total        real
instance_eval                                        9.090000   0.000000   9.090000 (  9.091285)
variable init                                        0.580000   0.000000   0.580000 (  0.579077)
instance variable get/set                            0.920000   0.000000   0.920000 (  0.915160)
current railties                                     1.160000   0.020000   1.180000 (  1.188045)

Benchmark/ips results:

==================================================
single `rake_task` method call inside loop
==================================================
Warming up --------------------------------------
       instance_eval     4.036k i/100ms
       variable init    39.834k i/100ms
    variable get/set    31.997k i/100ms
    current railties    35.253k i/100ms
Calculating -------------------------------------
       instance_eval     50.476k (±32.1%) i/s -    185.656k in   5.792784s
       variable init    977.498k (±56.9%) i/s -      1.315M in   5.415631s
    variable get/set    651.734k (±48.1%) i/s -      1.056M in   5.033967s
    current railties      1.417M (±37.0%) i/s -      1.128M in   7.740616s
Comparison:
    current railties:  1416776.9 i/s
       variable init:   977498.0 i/s - same-ish: difference falls within error
    variable get/set:   651733.6 i/s - same-ish: difference falls within error
       instance_eval:    50476.3 i/s - 28.07x slower
==================================================
Single `generators` method call inside loop
==================================================
Warming up --------------------------------------
       instance_eval     3.460k i/100ms
       variable init    59.450k i/100ms
    variable get/set    39.731k i/100ms
    current railties    50.625k i/100ms
Calculating -------------------------------------
       instance_eval     56.878k (±24.7%) i/s -    214.520k in   5.362425s
       variable init      1.114M (±53.8%) i/s -      1.784M in   5.180813s
    variable get/set    735.448k (±38.6%) i/s -      1.391M in   5.983204s
    current railties      1.108M (±60.7%) i/s -      1.063M in   5.020838s
Comparison:
       variable init:  1114123.1 i/s
    current railties:  1108297.5 i/s - same-ish: difference falls within error
    variable get/set:   735447.9 i/s - same-ish: difference falls within error
       instance_eval:    56878.4 i/s - 19.59x slower
==================================================
4 methods calls inside each loop
==================================================
Warming up --------------------------------------
       instance_eval     1.159k i/100ms
       variable init    25.411k i/100ms
    variable get/set    17.967k i/100ms
    current railties    19.728k i/100ms
Calculating -------------------------------------
       instance_eval     14.180k (±29.9%) i/s -     53.314k in   5.469904s
       variable init    377.365k (±53.4%) i/s -    736.919k in   6.285703s
    variable get/set    222.860k (±41.0%) i/s -    521.043k in   5.979247s
    current railties    504.417k (±42.5%) i/s -    493.200k in   5.210466s
Comparison:
    current railties:   504417.5 i/s
       variable init:   377364.7 i/s - same-ish: difference falls within error
    variable get/set:   222860.0 i/s - same-ish: difference falls within error
       instance_eval:    14180.2 i/s - 35.57x slower

@fidelisrafael fidelisrafael changed the title from Added register_block method to register rake_tasks, generators, and others blocks... to [railties] Added register_block method to register rake_tasks, generators, and others blocks... Aug 4, 2016

@fidelisrafael fidelisrafael changed the title from [railties] Added register_block method to register rake_tasks, generators, and others blocks... to [railties] Added register_block_for method to register Railties engines blocks(eg: rake_tasks, generators) Aug 4, 2016

@maclover7 maclover7 added the railties label Aug 4, 2016

@fidelisrafael fidelisrafael changed the title from [railties] Added register_block_for method to register Railties engines blocks(eg: rake_tasks, generators) to Added register_block_for method to register Railties engines blocks(eg: rake_tasks, generators) Aug 5, 2016

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Aug 17, 2016

Member

Could you squash your commits and rebase the branch?

Member

rafaelfranca commented Aug 17, 2016

Could you squash your commits and rebase the branch?

@rafaelfranca rafaelfranca self-assigned this Aug 17, 2016

@arthurnn

This comment has been minimized.

Show comment
Hide comment
@arthurnn

arthurnn Nov 1, 2016

Member

thanks @fidelisrafael
this was merged into master via 4a8edc0

Member

arthurnn commented Nov 1, 2016

thanks @fidelisrafael
this was merged into master via 4a8edc0

@arthurnn arthurnn closed this Nov 1, 2016

arthurnn added a commit that referenced this pull request Nov 1, 2016

@fidelisrafael

This comment has been minimized.

Show comment
Hide comment
@fidelisrafael

fidelisrafael Nov 1, 2016

Contributor

@arthurnn Cool! Thanks for letting me know. o/

Contributor

fidelisrafael commented Nov 1, 2016

@arthurnn Cool! Thanks for letting me know. o/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment