Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Improves performance of String#[] method when passing Fixnum. #1027

Merged
merged 1 commit into from

2 participants

@pgrzegorczyk

Hi,
this patch is related to issue #985. CSV.rb base on String#[] method to iterate through entire string object.
Before applying this patch running this code:

require 'csv'
CSV.foreach 'test.csv' do |row|
end

on 31MB file takes:

  • ruby 1.9.2-p180:
    ruby test.rb 8,96s user 0,06s system 99% cpu 9,028 total

  • ruby 1.8.7-p334:
    ruby test.rb 197,54s user 0,52s system 99% cpu 3:18,26 total

  • rubinius 1.2.4dev (1.8.7 5cb7a7d yyyy-mm-dd JI):
    ruby test.rb 168,26s user 0,60s system 100% cpu 2:48,36 total

  • rubinius-patched:
    ruby test.rb 116,09s user 0,40s system 100% cpu 1:55,85 total

After applying this patch it takes ~30% less time to complete.

@pgrzegorczyk pgrzegorczyk Improves performance of String#[] method.
Improves performance of String#[] method when passing Fixnum.
83341ab
@hosiawak
Collaborator

Running the ips benchmark before applying the path:

karol@mint ~/projects/personal/rubinius $ ./bin/benchmark benchmark/core/string/bench_slice.rb 
=== bin/rbx ===
       slice(fixnum)   767987.7 (±0.7%) i/s -    3850369 in   5.013849s (cycle=20159)
slice(fixnum, fixnum)
                       811764.0 (±0.8%) i/s -    4064125 in   5.006862s (cycle=19825)
       slice.(range)   451571.2 (±0.6%) i/s -    2269950 in   5.026942s (cycle=20450)
        slice(regex)   174882.9 (±4.9%) i/s -     873840 in   5.010468s (cycle=10923)
    slice(regexp, 1)   132964.6 (±4.6%) i/s -     661546 in   4.987545s (cycle=8374)
slice(other_str), matches
                       553490.1 (±3.6%) i/s -    2770304 in   5.013028s (cycle=21643)
slice(other_str), no matches
                       470645.2 (±0.4%) i/s -    2357468 in   5.009099s (cycle=20323)
karol@mint ~/projects/personal/rubinius $ ./bin/benchmark benchmark/core/string/bench_slice.rb 
=== bin/rbx ===
       slice(fixnum)   701172.4 (±5.4%) i/s -    3495540 in   5.001095s (cycle=20562)
slice(fixnum, fixnum)
                       817458.4 (±0.5%) i/s -    4089120 in   5.002387s (cycle=19472)
       slice.(range)   461322.3 (±0.3%) i/s -    2309268 in   5.005816s (cycle=20436)
        slice(regex)   170744.4 (±4.6%) i/s -     850824 in   4.994851s (cycle=10908)
    slice(regexp, 1)   130949.3 (±7.6%) i/s -     652158 in   5.012542s (cycle=8361)
slice(other_str), matches
                       595505.2 (±0.6%) i/s -    2972914 in   4.992454s (cycle=22694)
slice(other_str), no matches
                       477724.8 (±0.4%) i/s -    2382538 in   4.987324s (cycle=20191)

Patch applied:

karol@mint ~/projects/personal/rubinius $ ./bin/benchmark benchmark/core/string/bench_slice.rb 
=== bin/rbx ===
       slice(fixnum)  1059339.0 (±1.1%) i/s -    5299450 in   5.003393s (cycle=20150)
slice(fixnum, fixnum)
                       906644.9 (±0.8%) i/s -    4541988 in   5.009966s (cycle=19921)
       slice.(range)   428008.2 (±1.1%) i/s -    2136024 in   4.991222s (cycle=19778)
        slice(regex)   169317.3 (±4.7%) i/s -     846132 in   5.010232s (cycle=10073)
    slice(regexp, 1)   134581.6 (±4.2%) i/s -     670740 in   4.993965s (cycle=7985)
slice(other_str), matches
                       529825.0 (±0.9%) i/s -    2642616 in   4.988122s (cycle=20808)
slice(other_str), no matches
                       451331.0 (±0.9%) i/s -    2261130 in   5.010306s (cycle=20010)
karol@mint ~/projects/personal/rubinius $ ./bin/benchmark benchmark/core/string/bench_slice.rb 
=== bin/rbx ===
       slice(fixnum)  1067298.2 (±0.4%) i/s -    5354720 in   5.017143s (cycle=21856)
slice(fixnum, fixnum)
                       889257.2 (±0.8%) i/s -    4452051 in   5.006795s (cycle=20329)
       slice.(range)   435109.3 (±1.6%) i/s -    2182265 in   5.016859s (cycle=20395)
        slice(regex)   167025.9 (±4.5%) i/s -     835146 in   5.011756s (cycle=10707)
    slice(regexp, 1)   132173.1 (±4.3%) i/s -     664686 in   5.039547s (cycle=8206)
slice(other_str), matches
                       533023.0 (±0.6%) i/s -    2674493 in   5.017759s (cycle=21059)
slice(other_str), no matches
                       457413.4 (±0.4%) i/s -    2289168 in   5.004669s (cycle=20439)

It is indeed about 30% faster for the most common case. The question is do we still need the else block ?

I did not removed the else block because in method []= there is similar optimisation and there this block is kept.

@hosiawak hosiawak merged commit bbb18ed into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 18, 2011
  1. @pgrzegorczyk

    Improves performance of String#[] method.

    pgrzegorczyk authored
    Improves performance of String#[] method when passing Fixnum.
This page is out of date. Refresh to see the latest.
Showing with 9 additions and 0 deletions.
  1. +9 −0 kernel/common/string.rb
View
9 kernel/common/string.rb
@@ -254,6 +254,15 @@ def [](index, other = undefined)
end
case index
+ when Fixnum
+ # The same code as in else section.
+ # Copied here to improve performance because Fixnum index is
+ # often used to iterate through String object.
+ index = Rubinius::Type.coerce_to index, Fixnum, :to_int
+ index = @num_bytes + index if index < 0
+
+ return if index < 0 || @num_bytes <= index
+ return @data[index]
when Regexp
match_data = index.search_region(self, 0, @num_bytes, true)
Regexp.last_match = match_data
Something went wrong with that request. Please try again.