From: Ryan Davis <ryand-ruby@...>
Date: 2009-11-16T13:05:21+09:00
Subject: [ruby-core:26758] Re: (send #2) cgi.rb cleanup


On Nov 15, 2009, at 19:37 , Fujioka wrote:

> Hi matz and ryan
> 
> I can't agree this patch.
> Because, this patch is dropped performance.

OK. I've rolled it out of 1.8, working on 1.9 now. [1.9 now done]

The current code seems very odd (setting hash default to [] is just wrong). I've micro-benchmarked the hash/push operation and while the worst case (all uniq) scenario is indeed slower, the best case (all dupes) is faster than your implementation. But I have no idea how much of a bottleneck any of this would be in real world performance. Given that this is read_multipart, I'd think it was a minor part of any existing bottlenecks. I'd rather have the clearer code, but that is for you to decide.

dups = 75% duplication (25 params * 4)

> 511 % ./hash_time.rb 100000
> # of iterations = 100000
>                           user     system      total        real
> null_time             0.010000   0.000000   0.010000 (  0.010653)
> null_each             1.520000   0.000000   1.520000 (  1.528634)
> has_key? uniq        15.990000   0.070000  16.060000 ( 16.447104)
> has_key? dups        13.280000   0.220000  13.500000 ( 15.760251)
> h[k] = [] uniq       29.860000   0.470000  30.330000 ( 30.533542)
> h[k] = [] dups       13.020000   0.130000  13.150000 ( 13.163339)

dups == 100% duplication (1 params * 100)

> 512 % ./hash_time.rb 100000
> # of iterations = 100000
>                           user     system      total        real
> null_time             0.010000   0.000000   0.010000 (  0.010670)
> null_each             1.510000   0.000000   1.510000 (  1.508441)
> has_key? uniq        15.390000   0.020000  15.410000 ( 15.411080)
> has_key? dups        10.310000   0.040000  10.350000 ( 10.454800)
> h[k] = [] uniq       32.520000   0.070000  32.590000 ( 32.657144)
> h[k] = [] dups        7.910000   0.050000   7.960000 (  7.963789)

I can provide the benchmark code... but again, it is just a micro-bench.

Sorry for the noise.