Project

General

Profile

Actions

Misc #10233

closed

iseq.cのget_line_info()でバイナリサーチを使うように修正するか

Misc #10233: iseq.cのget_line_info()でバイナリサーチを使うように修正するか

Added by shiracha (Rikyu Shiracha) about 11 years ago. Updated almost 7 years ago.

Status:
Closed
Assignee:
-
[ruby-dev:48536]

Description

以下のPullRequestを投げさせていただきました。
https://github.com/ruby/ruby/pull/712

TODOコメントがあって気になったのでその通りの修正をとりあえず作ってみた形です。

ただむしろlinear searchのままにしておいて、コメントを
「ここは主にエラーハンドリングやデバッグに利用される部分でシンプルに保ちたいのでlinear searchを使っている」
という形に修正するべきかもしれない。とも思って迷っており、やはり辞めるべきかレビューを頂きたいです。


Files

benchmark_short.rb (1.5 KB) benchmark_short.rb shiracha (Rikyu Shiracha), 09/13/2014 06:46 AM
benchmark_long.rb (7.65 KB) benchmark_long.rb shiracha (Rikyu Shiracha), 09/13/2014 06:46 AM
benchmark_log.txt (1.69 KB) benchmark_log.txt shiracha (Rikyu Shiracha), 09/13/2014 06:46 AM

Updated by ko1 (Koichi Sasada) about 11 years ago Actions #1 [ruby-dev:48537]

ありがとうございます。
性能評価みたいなことって出来ますか?
(短いと、実は、線形サーチのほうが速かったりして)

Updated by nobu (Nobuyoshi Nakada) about 11 years ago Actions #2 [ruby-dev:48539]

とりあえずインデントは揃えてください。

Updated by shiracha (Rikyu Shiracha) about 11 years ago Actions #3 [ruby-dev:48540]

Koichi Sasada wrote:

ありがとうございます。
性能評価みたいなことって出来ますか?
(短いと、実は、線形サーチのほうが速かったりして)

これでよいかちょっとわからないのですが、一応とってみました。
添付のbenchmark_short/long.rbをパッチとパッチを当てる直前のコミットで比較してあります。
結果がbenchmark_log.txtです。

内容としては例外が主な用途だろうということでその場合の動作で見ています。

特に有意差はなさそうな感じですね。。。複雑になるだけかもしれない。

Nobuyoshi Nakada wrote:

とりあえずインデントは揃えてください。

すいません!ammendコミットをforce pushして直しておきました。
(最初はタブとスペースが混ざったとても変わったフォーマッティングだと思ってそのままの見た目にしたがってしまったんですが、
改めて読むと4spaceでインデント合わせしてるけどtabstopは8で、コード頭のほうのスペースが8になったらタブに置き換えるスタイルだったんですね...
vimでいうと noexpandtab tabstop=8。)

Updated by k0kubun (Takashi Kokubun) almost 7 years ago Actions #4 [ruby-dev:50771]

  • Status changed from Open to Closed

たまたまGitHubで見かけて、現状のtrunkには適用できない状態になっていたので、closeします。

Updated by mame (Yusuke Endoh) almost 7 years ago Actions #5 [ruby-dev:50774]

このチケットを全く知りませんでした(すみません)が、2.5 で二分探索にしました。さらにその後 2.6 では簡潔ビットベクトルで O(1) にしました。

Actions

Also available in: PDF Atom