Project

General

Profile

Actions

Feature #13943

closed

Use unpack1 instead of unpack(template)[0] in erb.rb

Added by znz (Kazuhiro NISHIYAMA) over 6 years ago. Updated over 6 years ago.

Status:
Closed
Target version:
-
[ruby-core:83027]

Description

I think unpack1(template) is faster than unpack(template)[0].

Attached patch for lib/erb.rb.


Files

erb-unpack1.diff (355 Bytes) erb-unpack1.diff znz (Kazuhiro NISHIYAMA), 09/27/2017 12:52 PM

Updated by k0kubun (Takashi Kokubun) over 6 years ago

Could you put benchmark result?

Updated by wyhaines (Kirk Haines) over 6 years ago

It should implicitly be faster, as it avoids the array creation, insertion, and dereferencing that occurs with:

m.unpack("C")[0]

However, a benchmark to quantify the difference would be nice to have.

Updated by shevegen (Robert A. Heiler) over 6 years ago

I think a benchmark would be great. There are some ruby hackers/devs that almost always add benchmarks to their changes, which I think is nice.

Updated by znz (Kazuhiro NISHIYAMA) over 6 years ago

Using benchmark_driver gem, unpack1(template) is about 1.07x faster than unpack(template)[0].

% cat url_encode.yml
prelude: |
  require 'erb'
  str = "Programming Ruby:  The Pragmatic Programmer's Guide"
benchmark: ERB::Util.url_encode(str)
% benchmark_driver url_encode.yml -e ruby-base::$(rbenv root)/versions/git/bin/ruby -e ruby-unpack1::$(rbenv root)/versions/git2/bin/ruby
benchmark results:
Execution time (sec)
name             ruby-base ruby-unpack1
url_encode       1.793    1.665

Speedup ratio: compare with the result of `ruby-base' (greater is better)
name             ruby-unpack1
url_encode       1.077

Updated by k0kubun (Takashi Kokubun) over 6 years ago

  • Assignee changed from k0kubun (Takashi Kokubun) to znz (Kazuhiro NISHIYAMA)

Thank you for showing benchmark result.
While the impact is not so large with that benchmark, this would be expected usage of #unpack1 and it's good for maintainability too. So I think "not being slower in benchmark" is enough for adding this change.
I'm +1 for committing this.

Actions #6

Updated by Anonymous over 6 years ago

  • Status changed from Assigned to Closed

Applied in changeset trunk|r60061.


Use unpack1 instead of unpack and [0]

[Feature #13943][ruby-core:83027]

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0