Project

General

Profile

Feature #6082

io_binwrite()内でwritev()を使う

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

Status:
Rejected
Priority:
Normal
Target version:
-
[ruby-dev:45285]

Description

io.cのio_binwrite()内で、writev()を用いる事を提案します。

現在のio_binwrite()は、実際にシステムコールを呼ぶ時には渡された文字列がwbufに収まるならばMEMMOVE()で収めてしまってio_fflush()し、
収まらなければfflushした後に再度rb_write_internal()を呼んでいますが、writev()を用いる事でMEMMOVE()の必要がなくなり、
wbufと渡された文字列を結合する事なく一度のシステムコールで書き出す事ができます。
また、wbufと渡された文字列を1度に書き出す事に成功すれば、他のスレッドからのwriteが間に入らないというメリットもあります。

patchを添付します。
test-allを実行したところ、test_keep_alive_EOF(TestNetHTTPKeepAlive)でfailure、test_inspect_nonascii(TestDir_M17N)でerrorとなりますが、
これはtrunkのr34799でも同様なので、このpatchによる問題ではないものと思われます。


Files

patch.diff (4.32 KB) patch.diff Glass_saga (Masaki Matsushita), 02/25/2012 01:33 PM
patch2.diff (5.28 KB) patch2.diff Glass_saga (Masaki Matsushita), 02/26/2012 11:30 AM
shift_fflush.diff (6.26 KB) shift_fflush.diff #ifndef HAVE_WRITEVな場合にmemmove()とfflushを担当するio_shift_fflush()という関数を新設して#ifdefを小さくしたpatch Glass_saga (Masaki Matsushita), 04/02/2012 10:17 PM
binwrite.diff (6.49 KB) binwrite.diff io_binwrite_string()にwbufのwriteと引数ptrのwriteの両方を担当させる事で#ifdefを小さくしたpatch Glass_saga (Masaki Matsushita), 04/02/2012 10:17 PM

Updated by nobu (Nobuyoshi Nakada) almost 8 years ago

なかだです。

(12/02/25 13:33), Masaki Matsushita wrote:

patchを添付します。

sys/uio.hが必要です。

diff --git i/configure.in w/configure.in
index c760c53..67f5a54 100644
--- i/configure.in
+++ w/configure.in
@@ -1144,7 +1144,7 @@ AC_CHECK_HEADERS(limits.h sys/file.h sys/ioctl.h sys/syscall.h\
syscall.h pwd.h grp.h a.out.h utime.h direct.h sys/resource.h \
sys/mkdev.h sys/utime.h xti.h netinet/in_systm.h float.h ieeefp.h \
ucontext.h intrinsics.h langinfo.h locale.h sys/sendfile.h time.h \

  • net/socket.h sys/socket.h process.h sys/prctl.h)
  • net/socket.h sys/socket.h process.h sys/prctl.h sys/uio.h)

AC_TYPE_SIZE_T
RUBY_CHECK_SIZEOF(size_t, [int long void*], [], [@%:@include ])
diff --git i/io.c w/io.c
index bbaf72c..3173d85 100644
--- i/io.c
+++ w/io.c
@@ -78,6 +78,10 @@
#include
#endif

+#ifdef HAVE_SYS_UIO_H
+#include
+#endif
+
#if defined(BEOS) || defined(HAIKU)
# ifndef NOFILE
# define NOFILE (OPEN_MAX)
@@ -9452,10 +9456,6 @@ simple_sendfile(int out_fd, int in_fd, off_t *offset, off_t count)
*/
# define USE_SENDFILE

-# ifdef HAVE_SYS_UIO_H
-# include
-# endif
-
static ssize_t
simple_sendfile(int out_fd, int in_fd, off_t *offset, off_t count)
{

--
--- 僕の前にBugはない。
--- 僕の後ろにBugはできる。
中田 伸悦

Updated by Glass_saga (Masaki Matsushita) almost 8 years ago

Nobuyoshi Nakada wrote:

sys/uio.hが必要です。

ご指摘とpatchをありがとうございます。
なかださんのpatchを適用して1つにまとめたpatchを添付します。

Updated by mame (Yusuke Endoh) almost 8 years ago

  • Status changed from Open to Assigned
  • Assignee set to nobu (Nobuyoshi Nakada)

分かってませんが挙動が変わるものでないなら取り込んじゃってよいのでは
ないでしょうかね。何かの環境で問題起きたら revert で。

なかださんお願いします。
なんとなく kosaki さんあたり、もしコメントあったらどうぞ。

--
Yusuke Endoh mame@tsg.ne.jp

Updated by kosaki (Motohiro KOSAKI) almost 8 years ago

なんとなく kosaki さんあたり、もしコメントあったらどうぞ。

関数のなかにでっかく#ifdefいれんな。というのが1つ。writevがない環境を考えると既存のコードを消せないので有為に速度向上することが提示されないと入れる意味が無いというのが1つ。そんぐらいかなあ

Updated by mame (Yusuke Endoh) almost 8 years ago

  • Status changed from Assigned to Feedback
  • Assignee changed from nobu (Nobuyoshi Nakada) to Glass_saga (Masaki Matsushita)

あ、このチケットにだけベンチマークがないのか。他には執拗なくらいあるから勘違いしました。
パッチについてもそうですね。じゃあ、Glass_saga さん 2 点対応してみてもらえますか。

--
Yusuke Endoh mame@tsg.ne.jp

Updated by Glass_saga (Masaki Matsushita) almost 8 years ago

次のようなベンチマークを実行してみましたが、結果にばらつきはあるものの意味のある高速化にはならなかったようです。

require 'benchmark'
require 'tempfile'

str = "a" * 8000

Tempfile.open("foo") do |f|
Benchmark.bm do |x|
x.report do
20000.times { f.write str }
end
end
end

trunk(r35215):
user system total real
0.070000 0.240000 0.310000 ( 0.346534)
user system total real
0.110000 0.270000 0.380000 ( 0.418857)
user system total real
0.070000 0.270000 0.340000 ( 0.512318)

proposal:
user system total real
0.050000 0.240000 0.290000 ( 0.363697)
user system total real
0.050000 0.220000 0.270000 ( 0.428360)
user system total real
0.080000 0.260000 0.340000 ( 0.518545)

但し、modeにFile::SYNCを指定している場合には格段に速くなるようです。

require 'benchmark'
require 'tempfile'

str = "a" * 8000

Tempfile.open("foo", nil, nil,{:mode => File::SYNC}) do |f|
Benchmark.bm do |x|
x.report do
1000.times { f.write str }
end
end
end

trunk(r35215):
user system total real
0.020000 0.280000 0.300000 ( 1.452105)
user system total real
0.010000 0.270000 0.280000 ( 1.452677)
user system total real
0.010000 0.220000 0.230000 ( 1.348059)

proposal:
user system total real
0.010000 0.120000 0.130000 ( 0.714809)
user system total real
0.020000 0.120000 0.140000 ( 0.669088)
user system total real
0.010000 0.210000 0.220000 ( 0.833870)

しかし、同期モードでの書き込みが速くなる事にどれほどの意義があるのかはわかりません。

straceで見るとproposalでは確かにwritev()が使われているので、trunkにおいてmemmove()やシステムコールの呼び出しにかかるコストと、
proposalにおいてiovec構造体の配列を造るのにかかるコストが同程度な為に、同期モードでない場合には高速化に至らなかったものと思われます。
始めからきちんとベンチマークを取っていれば良かったのですが、お騒がせしました。

#ifdefを小さくする為に書いた2つのpatchをご参考までに添付します(速度面ではどちらも変わりません)。
「ここがおかしいから直せば速くなる」などありましたら指摘して下さると幸いです。

Updated by mame (Yusuke Endoh) almost 8 years ago

  • Status changed from Feedback to Rejected

あまり意義がなさそうとのことなので、とりあえず閉じます。
同期モードで高速化してほしいユースケースはあるとか、その他何か意見があったら
reopen してください。

--
Yusuke Endoh mame@tsg.ne.jp

Also available in: Atom PDF