Skip to content
  • Jeremy Evans's avatar
    c20e819e
    Fix crash when passing large keyword splat to method accepting keywords and keyword splat · c20e819e
    Jeremy Evans authored
    The following code previously caused a crash:
    
    ```ruby
    h = {}
    1000000.times{|i| h[i.to_s.to_sym] = i}
    def f(kw: 1, **kws) end
    f(**h)
    ```
    
    Inside a thread or fiber, the size of the keyword splat could be much smaller
    and still cause a crash.
    
    I found this issue while optimizing method calling by reducing implicit
    allocations.  Given the following code:
    
    ```ruby
    def f(kw: , **kws) end
    kw = {kw: 1}
    f(**kw)
    ```
    
    The `f(**kw)` call previously allocated two hashes callee side instead of a
    single hash.  This is because `setup_parameters_complex` would extract the
    keywords from the keyword splat hash to the C stack, to attempt to mirror
    the case when literal keywords are passed without a keyword splat.  Then,
    `make_rest_kw_hash` would build a new hash based on the extracted keywords
    that weren't used for literal keywords.
    
    Switch the implementation so that if a keyword splat is passed, literal keywords
    are deleted from the keyword splat hash (or a copy of the hash if the hash is
    not mutable).
    
    In addition to avoiding the crash, this new approach is much more
    efficient in all cases.  With the included benchmark:
    
    ```
                                    1
                miniruby:   5247879.9 i/s
         miniruby-before:   2474050.2 i/s - 2.12x  slower
    
                            1_mutable
                miniruby:   1797036.5 i/s
         miniruby-before:   1239543.3 i/s - 1.45x  slower
    
                                   10
                miniruby:   1094750.1 i/s
         miniruby-before:    365529.6 i/s - 2.99x  slower
    
                           10_mutable
                miniruby:    407781.7 i/s
         miniruby-before:    225364.0 i/s - 1.81x  slower
    
                                  100
                miniruby:    100992.3 i/s
         miniruby-before:     32703.6 i/s - 3.09x  slower
    
                          100_mutable
                miniruby:     40092.3 i/s
         miniruby-before:     21266.9 i/s - 1.89x  slower
    
                                 1000
                miniruby:     21694.2 i/s
         miniruby-before:      4949.8 i/s - 4.38x  slower
    
                         1000_mutable
                miniruby:      5819.5 i/s
         miniruby-before:      2995.0 i/s - 1.94x  slower
    ```
    c20e819e
    Fix crash when passing large keyword splat to method accepting keywords and keyword splat
    Jeremy Evans authored
    The following code previously caused a crash:
    
    ```ruby
    h = {}
    1000000.times{|i| h[i.to_s.to_sym] = i}
    def f(kw: 1, **kws) end
    f(**h)
    ```
    
    Inside a thread or fiber, the size of the keyword splat could be much smaller
    and still cause a crash.
    
    I found this issue while optimizing method calling by reducing implicit
    allocations.  Given the following code:
    
    ```ruby
    def f(kw: , **kws) end
    kw = {kw: 1}
    f(**kw)
    ```
    
    The `f(**kw)` call previously allocated two hashes callee side instead of a
    single hash.  This is because `setup_parameters_complex` would extract the
    keywords from the keyword splat hash to the C stack, to attempt to mirror
    the case when literal keywords are passed without a keyword splat.  Then,
    `make_rest_kw_hash` would build a new hash based on the extracted keywords
    that weren't used for literal keywords.
    
    Switch the implementation so that if a keyword splat is passed, literal keywords
    are deleted from the keyword splat hash (or a copy of the hash if the hash is
    not mutable).
    
    In addition to avoiding the crash, this new approach is much more
    efficient in all cases.  With the included benchmark:
    
    ```
                                    1
                miniruby:   5247879.9 i/s
         miniruby-before:   2474050.2 i/s - 2.12x  slower
    
                            1_mutable
                miniruby:   1797036.5 i/s
         miniruby-before:   1239543.3 i/s - 1.45x  slower
    
                                   10
                miniruby:   1094750.1 i/s
         miniruby-before:    365529.6 i/s - 2.99x  slower
    
                           10_mutable
                miniruby:    407781.7 i/s
         miniruby-before:    225364.0 i/s - 1.81x  slower
    
                                  100
                miniruby:    100992.3 i/s
         miniruby-before:     32703.6 i/s - 3.09x  slower
    
                          100_mutable
                miniruby:     40092.3 i/s
         miniruby-before:     21266.9 i/s - 1.89x  slower
    
                                 1000
                miniruby:     21694.2 i/s
         miniruby-before:      4949.8 i/s - 4.38x  slower
    
                         1000_mutable
                miniruby:      5819.5 i/s
         miniruby-before:      2995.0 i/s - 1.94x  slower
    ```
Loading