Project

General

Profile

Actions

Feature #6218

closed

struct.cのrb_struct_s_members_m()について

Added by Glass_saga (Masaki Matsushita) almost 12 years ago. Updated almost 12 years ago.

Status:
Closed
Target version:
-
[ruby-dev:45451]

Description

表題の関数ではrb_struct_s_members()で得たArrayの内容をwhile文で1要素ずつ新たなArrayにpushしていますが、
これはrb_ary_dup()で済ませられるのではないでしょうか。

パフォーマンス上のメリットもあります。

require 'benchmark'

sym = :a
s = Struct.new(*Array.new(100){ sym = sym.succ })

Benchmark.bm do |x|
x.report do
1000.times { s.members }
end
end

以上のコードを実行したところ、以下の結果となりました。

trunk(r35158):
user system total real
0.000000 0.000000 0.000000 ( 0.003188)

proposal:
user system total real
0.000000 0.000000 0.000000 ( 0.000688)

patchを添付します。


Files

patch.diff (563 Bytes) patch.diff Glass_saga (Masaki Matsushita), 03/28/2012 10:24 PM

Updated by mame (Yusuke Endoh) almost 12 years ago

  • Status changed from Open to Assigned
  • Assignee set to Glass_saga (Masaki Matsushita)

なんで今そういう無駄なコードになっているか、というのを調べるといいです。
ひょっとしたら何か意味があるのかもしれないので。

この場合 r10847 を見ると、Struct#members が文字列からシンボルを返すように
変更したことで、こういうコードになったようです。
なので、昔は意味があったけれど今は本当にただの無駄だと思われます。

コミット権もらえたらやっといてください。

--
Yusuke Endoh

Updated by mame (Yusuke Endoh) almost 12 years ago

  • Assignee changed from Glass_saga (Masaki Matsushita) to mame (Yusuke Endoh)

Glass_saga さんのコミッタ化にはもうちょっと時間がかかりそうみたいなので、
とりあえず私がやっちゃいます。

--
Yusuke Endoh

Actions #3

Updated by mame (Yusuke Endoh) almost 12 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r35753.
Masaki, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • struct.c (rb_struct_members): Refactoring. As Struct#members had
    returned an array of String, the old code was needed to convert
    Symbols to Strings. But it is almost unnecessary because the
    method now returns an array of Symbols. A patch by Masaki
    Matsushita <glass.saga at gmail dot com> [Feature #6218]
    [ruby-dev:45451]
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0