Skip to content

Conversation

@bbatsov
Copy link
Collaborator

@bbatsov bbatsov commented May 17, 2013

Replaced push/reverse with unshift. This is generally about 10 times
faster according to my simple benchmarks.

Replace push/reverse with unshift. This is generally about 10 times
faster according to my simple benchmarks.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0%) when pulling 6b184e7 on bbatsov:optimize-line-begins into beb08d0 on whitequark:master.

@whitequark
Copy link
Owner

This is really strange; unshift must relocate the complete contents of the array. I'd like to see the benchmarks you have used.

@bbatsov
Copy link
Collaborator Author

bbatsov commented May 17, 2013

The benchmark I used was simply pushing 10k strings into an array and reversing; pushing + destructive reverse; unshift;

require 'benchmark'

a = []

time = Benchmark.realtime do
  10000.times do |i|
    a << "top #{i}"
    a = a.reverse
  end
end

puts time

This took 1.1 seconds. With reverse! it was 0.9. With unshift - 0.12. Benchmark is fairly primitive, but the results seem sensible to me. It simply seems that reverse is pretty expensive.

@whitequark
Copy link
Owner

See also https://gist.github.com/whitequark/a70f0d6fcaaf3a1cda0a and http://rxr.whitequark.org/mri/source/array.c#1031. This result is somewhat strange to me.

@bbatsov, does rubycop spend significant time in this function?

whitequark added a commit that referenced this pull request May 17, 2013
Optimize performance of Buffer#line_begins
@whitequark whitequark merged commit 0ea8523 into whitequark:master May 17, 2013
@bbatsov
Copy link
Collaborator Author

bbatsov commented May 17, 2013

Haven't checked. I was just reading Parser's source and noticed this could be optimized.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants