Project

General

Profile

Actions

Feature #5562

closed

Improvement of Windows IO performance

Added by h.shirosaki (Hiroshi Shirosaki) over 12 years ago. Updated over 12 years ago.

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

Description

=begin
I suggest a patch to improve Windows IO performance.

Ruby's text mode IO is much slower than binary mode.
On Windows text mode is default, so Windows IO is slow.
I assume that's mainly because of CRLF linefeed code conversion.

My idea to improve IO performance is as below.

  • Change default linefeed conversion from Universal newline to CRLF newline on Windows
  • Use binary mode process with OS's text mode if only CRLF conversion is needed
  • Use Ruby's text mode with universal newline conversion if encoding conversion is needed

Although that causes io.c code to be more complicated, IO with CRLF conversion performance seems to be improved much.
I confirmed "make test-all TEST=ruby" have been passed. There was 3 errors, but ruby without this patch had same errors.
I think this patch doesn't affect other OS.

Line endings of "p" or "puts" writing is LF on trunk, but CRLF on 1.8.7 or 1.9.2.
This patch reverts to CRLF.

Here is #1332 benchmark test and results.

time = [Time.new]
c = ''
'aaaa'.upto('zzzz') {|e| c << e}
4.times { c << c }
time << Time.new
File.open('out.file','w') { |f| f.write(c) }
time << Time.new
c = File.open('out.file','r') { |f| f.read }
time << Time.new
0.upto(time.size - 2) {|i| p "#{i} #{time[i+1]-time[i]}" }

  • Result

ruby 1.8.7 (2011-06-30 patchlevel 352) [i386-mingw32]
"0 0.78125"
"1 0.6875"
"2 0.5625"

ruby 2.0.0dev (2011-11-03) [i386-mingw32]
"0 0.59375"
"1 1.09375"
"2 1.296875"

ruby 2.0.0dev (2011-11-03 trunk 33615) [i386-mingw32] with this patch
"0 0.625"
"1 0.65625"
"2 0.34375"
=end


Files

win_io.patch (12.2 KB) win_io.patch h.shirosaki (Hiroshi Shirosaki), 11/04/2011 12:02 AM
test-all.txt (24.1 KB) test-all.txt h.shirosaki (Hiroshi Shirosaki), 11/04/2011 06:40 PM
win_io_for_r33656.patch (13.6 KB) win_io_for_r33656.patch h.shirosaki (Hiroshi Shirosaki), 11/08/2011 07:16 AM
test_result.tar.gz (154 KB) test_result.tar.gz h.shirosaki (Hiroshi Shirosaki), 11/11/2011 01:23 AM
win_io_for_r33699.patch (14 KB) win_io_for_r33699.patch h.shirosaki (Hiroshi Shirosaki), 11/11/2011 01:23 AM
win_io_for_r33908.patch (15.5 KB) win_io_for_r33908.patch h.shirosaki (Hiroshi Shirosaki), 11/30/2011 11:24 PM

Related issues 1 (0 open1 closed)

Related to Backport193 - Backport #5704: Please backport r33937 (newline decorator)Closedusa (Usaku NAKAMURA)12/04/2011Actions

Updated by usa (Usaku NAKAMURA) over 12 years ago

Did you run make test and make test-all?

Updated by Anonymous over 12 years ago

Ruby's text mode IO is much slower than binary mode.

Why though, is my question. It would be nice to get to the root of
the problem I suppose...

Updated by h.shirosaki (Hiroshi Shirosaki) over 12 years ago

"make test" have been passed with no fails.
"make test-all" had failures and errors.

I think below two cases are related to this patch.
I'm not sure about other failures and errors. Some libraries might lack in my environment.
I attached a test-all log.

  1. Failure:
    test_include_file_encoding_incompatible(TestRDocMarkupPreProcess) [c:/devruby/ru
    by/test/rdoc/test_rdoc_markup_pre_process.rb:75]:
    --- expected
    +++ actual
    @@ -1,2 +1,2 @@
    -"?
    +"?\r
    "

  2. Failure:
    test_include_file(TestRDocMarkupPreProcess) [c:/devruby/ruby/test/rdoc/test_rdoc
    _markup_pre_process.rb:51]:
    --- expected
    +++ actual
    @@ -1,3 +1,3 @@
    -"Regular expressions (regexps) are patterns which describe the
    -contents of a string.
    +"Regular expressions (regexps) are patterns which describe the\r
    +contents of a string.\r
    "

Updated by jonforums (Jon Forums) over 12 years ago

I've downloaded your patch and will try with both ruby_1_9_3 and trunk.

In the past, I've had many failures when running make test-all. I don't believe they've been resolved so I usually run make test && make test-all TESTS='openssl fiddle psych" after building. I'll try again and see how my results compare to your test-all log.

I assume the patch still works as intended since #33629 was applied.

  • Which version of MinGW GCC are you using to build?
  • Which version of Windows OS are you using?
  • Are you running Windows OS natively or on a VM like VirtualBox?

Thank you for making this patch; it looks very interesting.

Updated by h.shirosaki (Hiroshi Shirosaki) over 12 years ago

Thanks for trying this patch.

  • Which version of MinGW GCC are you using to build?
    4.5.2
  • Which version of Windows OS are you using?
    Windows XP SP3
  • Are you running Windows OS natively or on a VM like VirtualBox?
    natively

I used rubyinstaller's latest source code and built with "rake ruby19 LOCAL="c:/devruby/ruby".

Updated by h.shirosaki (Hiroshi Shirosaki) over 12 years ago

=begin
I'm sorry, previous comment failed to be submited. I don't know why.

  1. Failure:
    test_include_file_encoding_incompatible(TestRDocMarkupPreProcess) [c:/devruby/ru
    by/test/rdoc/test_rdoc_markup_pre_process.rb:75]:

  2. Failure:
    test_include_file_encoding_incompatible(TestRDocMarkupPreProcess) [c:/devruby/ru
    by/test/rdoc/test_rdoc_markup_pre_process.rb:75]:

It seems above failures are because tempfile binary read result are different.
Is trunk ruby behavior right?

$ ./ruby -v -I. -I../../../ruby/lib -rtempfile -e 'f = Tempfile.new("a"); f.write "a\nb"; f.close; o
pen(f.path, "rb") {|r| p r.read}'
ruby 2.0.0dev (2011-11-03) [i386-mingw32]
"a\nb"

with this patch

$ ./ruby -v -I. -I../../../ruby/lib -rtempfile -e 'f = Tempfile.new("a"); f.write "a\nb"; f.close; o
pen(f.path, "rb") {|r| p r.read}'
ruby 2.0.0dev (2011-11-03 trunk 33615) [i386-mingw32]
"a\r\nb"

$ /c/Ruby192/bin/ruby -v -rtempfile -e 'f = Tempfile.new("a"); f.write "a\nb"; f.close; open(f.path,
"rb") {|r| p r.read}'
ruby 1.9.2p290 (2011-07-09) [i386-mingw32]
"a\r\nb"
=end

Updated by jonforums (Jon Forums) over 12 years ago

I am seeing a strange issue with the patch on my Win7 32bit system. I'm not yet sure whether it's my setup or a real issue.

In summary, although the patch applies cleanly (with offsets) and tests pass, at runtime I get This program cannot be run in DOS mode failures for ruby.exe. It appears ruby.exe, msvcrt-ruby191.dll, and others are being corrupted and not recognized as PE files. This occurs regardless of whether I build with RubyInstaller/MinGW or Windows SDK. Strangly, when I cross-compile on Arch and MinGW the binaries appear to be correctly built and execute on Win7.

Here's what I know so far:

  • failure occurs when building with Win7 on either ruby_1_9_3@33569 or trunk@33634
  • failure occurs using TDM-GCC 4.5.2/4.6.1 and Windows SDK for Windows 7
  • no failure when cross-compiled with Arch VM (VirtualBox) with i486-mingw32-gcc 4.6.1
  • miniruby.exe is not corrupted as both make test and make test-all complete
  • good msvcrt-ruby191.dll is 2,138,112 bytes, good ruby.exe is 73,728 bytes
  • bad msvcrt-ruby191.dll is 4,096 bytes, bad ruby.exe is 4,096 bytes

C:\Users\Jon\Documents\RubyDev\ruby-git>git apply --verbose win_io.patch

many messages about hunks applied with offsets

Applied patch include/ruby/encoding.h cleanly.
Applied patch io.c cleanly.
Applied patch win32/win32.c cleanly.

C:\Users\Jon\Documents\RubyDev\ruby-git>git st

trunk

M include/ruby/encoding.h
M io.c
M win32/win32.c

FAILURE - RubyInstaller build recipes using TDM GCC 4.5.2/4.6.1

C:\projects\rubyinstaller-git>rake ruby19 local=C:\Users\Jon\Documents\RubyDev\ruby-git nogems=1
...
mkdir -p sandbox/rubygems
mkdir -p sandbox/ruby19_mingw/lib/ruby/1.9.1/rubygems/defaults
cp resources/rubygems/operating_system.rb sandbox/ruby19_mingw/lib/ruby/1.9.1/rubygems/defaults
mkdir -p sandbox/rb-readline
"C:/projects/rubyinstaller-git/sandbox/extract_utils/7za.exe" x -y -o"sandbox/rb-readline" "C:/projects/rubyinstaller-git/downloads/rb-readline-0.4.2.zip" > NUL 2>&1
cd sandbox/rb-readline
ruby setup.rb
This program cannot be run in DOS mode.
rake aborted!
Command failed with status (1): [ruby setup.rb ...]

Tasks: TOP => ruby19 => dependencies:rbreadline:install19

C:\projects\rubyinstaller-git>rake devkit:sh
Temporarily enhancing PATH to include DevKit...
sh-3.1$ export PATH=/c/projects/rubyinstaller-git/sandbox/ruby19_mingw/bin:$PATH
sh-3.1$ ruby --version
This program cannot be run in DOS mode.
sh-3.1$ objdump -h bin/ruby.exe
C:\projects\rubyinstaller-git\sandbox\devkit\mingw\bin\objdump.exe: bin/ruby.exe: File format not recognized
sh-3.1$ objdump -h bin/msvcrt-ruby191.dll
C:\projects\rubyinstaller-git\sandbox\devkit\mingw\bin\objdump.exe: bin/msvcrt-ruby191.dll: File format not recognized
sh-3.1$ objdump -h lib/ruby/1.9.1/i386-mingw32/dl.so
C:\projects\rubyinstaller-git\sandbox\devkit\mingw\bin\objdump.exe: lib/ruby/1.9.1/i386-mingw32/dl.so: File format not recognized
sh-3.1$ objdump -h miniruby.exe

miniruby.exe: file format pei-i386

Sections:
Idx Name Size VMA LMA File off Algn
0 .text 001675fc 00401000 00401000 00000600 22
CONTENTS, ALLOC, LOAD, READONLY, CODE, DATA
1 .data 0000196c 00569000 00569000 00167c00 2
5
CONTENTS, ALLOC, LOAD, DATA
...
sh-3.1$ make test-all TESTS='openssl fiddle psych'
./miniruby.exe ...
...
684 tests, 3621 assertions, 0 failures, 0 errors, 1 skips

ruby -v: wio-ruby 2.0.0dev (2011-11-04 trunk 33634) [i386-mingw32]
sh-3.1$ make test
...
PASS all 943 tests
./miniruby.exe ...
Driver is wio-ruby 2.0.0dev (2011-11-04 trunk 33634) [i386-mingw32]
Target is wio-ruby 2.0.0dev (2011-11-04 trunk 33634) [i386-mingw32]

KNOWNBUGS.rb .
PASS all 1 tests

V:\sandbox\ruby19_mingw>dumpbin -summary bin\ruby.exe
Microsoft (R) COFF/PE Dumper Version 10.00.30319.01

Dump of file bin\ruby.exe
LINK : warning LNK4094: 'bin\ruby.exe' is an MS-DOS executable; use EXEHDR to dump it

V:\sandbox\ruby19_mingw>dumpbin -summary bin\msvcrt-ruby191.dll
Dump of file bin\msvcrt-ruby191.dll
LINK : warning LNK4094: 'bin\msvcrt-ruby191.dll' is an MS-DOS executable; use EXEHDR to dump it

FAILURE - Build with Windows SDK for Windows 7

C:\Users\Jon\DOCUME~1\RubyDev\ruby-git\build>..\win32\configure.bat --prefix=C:\projects\ruby200-mswin
--target=i686-mswin32 --disable-install-doc --disable-win95

C:\Users\Jon\DOCUME~1\RubyDev\ruby-git\build>nmake
...
C:\Users\Jon\DOCUME~1\RubyDev\ruby-git\build>nmake test
...
PASS all 943 tests
...
KNOWNBUGS.rb .
PASS all 1 tests
C:\Users\Jon\Documents\RubyDev\ruby-git\build>nmake install
...
C:\Users\Jon\Documents\RubyDev\ruby-git\build>\projects\ruby200-mswin\bin\ruby.exe --version
This program cannot be run in DOS mode.

C:\Users\Jon\Documents\RubyDev\ruby-git\build>dumpbin -summary \projects\ruby200-mswin\bin\ruby.exe
Microsoft (R) COFF/PE Dumper Version 10.00.30319.01

Dump of file \projects\ruby200-mswin\bin\ruby.exe
LINK : warning LNK4094: '\projects\ruby200-mswin\bin\ruby.exe' is an MS-DOS executable; use EXEHDR to dump it

Updated by jonforums (Jon Forums) over 12 years ago

FYI, unrelated to the patch failure on my system (but related to Windows IO performance) I've found a way to build a profileable MRI on Windows (basically optflags='-gstabs+' for MinGW builds) and analyze with the freely available AQtime Standard http://smartbear.com/products/free-tools/aqtime-standard/

Detailed build instructions at https://github.com/thecodeshop/ruby/wiki/Building-MRI-on-Windows on how to tweak the RubyInstaller build recipes.

Here's a screenshot of a profile result on a workload that performs a text read on a 500,000 line (LF delimited) test file

http://www.mediafire.com/imgbnc.php/dd9263c046e611fb09011894cd545fc80cbcbd177bb56c296aa986782b8c00346g.jpg

The specific workload I used was

https://github.com/jonforums/measurements/blob/master/workloads/core_rd_filelines_lf.rb

and is part of the benchmark/profiler/tracer helper tool (runs standalone or with pik or rvm on Windows and Linux) I wrote https://github.com/jonforums/measurements

If any of this looks helpful for what you're trying to do, please feel free to contact me off-list. There's also a small group of interested folks trying to looking at Windows performance improvement opportunities on the http://groups.google.com/group/thecodeshop ML if you want to post there.

And I bet there are others hard at work on optimizations ;)

Given what I think your patch is trying to do, and the current profiling results I seen, I really think your patch can make a very large improvement for Windows by getting much closer to full binary read performance on Linux. Very exciting, and I hope to quickly find a fix so I can start investigating your patch.

Jon


http://thecodeshop.github.com | http://jonforums.github.com/
twitter: @jonforums (Jon Forums)

Updated by jonforums (Jon Forums) over 12 years ago

Err...make that debugflags='-gstabs+'

Updated by usa (Usaku NAKAMURA) over 12 years ago

Hello,

In message "[ruby-core:40746] [ruby-trunk - Feature #5562] Improvement of Windows IO performance"
on Nov.05,2011 09:33:57, wrote:

  1. Failure:
    test_include_file_encoding_incompatible(TestRDocMarkupPreProcess) [c:/devruby/ru
    by/test/rdoc/test_rdoc_markup_pre_process.rb:75]:

  2. Failure:
    test_include_file_encoding_incompatible(TestRDocMarkupPreProcess) [c:/devruby/ru
    by/test/rdoc/test_rdoc_markup_pre_process.rb:75]:

It seems above failures are because tempfile binary read result are different.
Is trunk ruby behavior right?

These two failure is the problem of rdoc, not ruby's.
So you can ignore them.

Regards,

U.Nakamura

Updated by h.shirosaki (Hiroshi Shirosaki) over 12 years ago

Thank you for your help.

Previous patch have a bug of IO.copy_stream. So, many errors have occurred in rubygems and http tests.
I assume "This program cannot be run in DOS mode." is related to above issue. I had same error.
Does Ruby install process use IO.copy_stream?

I was able to fix that bugs. make test-all failures and errors have reduced, although this still have some failures and errors.
Now I can run ruby on rails with patched ruby.

Updated by jonforums (Jon Forums) over 12 years ago

Looks great so far. More info and profiling snapshot tomorrow, but here's what I get so far on my Win7 32bit:

C:\projects\rubyinstaller-git>rake ruby19 local=c:\Users\Jon\Documents\RubyDev\ruby-git nogems=1
...
sh-3.1$ make test-all TESTS='openssl fiddle psych'
...
638 tests, 3490 assertions, 0 failures, 0 errors, 1 skips
sh-3.1$ make test
PASS all 943 tests
...
KNOWNBUGS.rb .
PASS all 1 tests

Files used in micro-benchmarks below. Bottom line...with your patch, performance is much closer to binary-only read performance.

https://github.com/jonforums/measurements/blob/master/workloads/core_brd_filelines_lf.rb
https://github.com/jonforums/measurements/blob/master/workloads/core_rd_filelines_lf.rb

C:\projects\measurements-git>rci bench core_brd_filelines_lf
wio-ruby 1.9.3p0 (2011-11-08 revision 33661) [i386-mingw32]
Rehearsal ---------------------------------------------------------
core_brd_filelines_lf 0.827000 0.141000 0.968000 ( 0.967202)
------------------------------------------------ total: 0.968000sec

                        user     system      total        real

core_brd_filelines_lf 0.811000 0.171000 0.982000 ( 0.998401)

C:\projects\measurements-git>rci bench core_rd_filelines_lf
wio-ruby 1.9.3p0 (2011-11-08 revision 33661) [i386-mingw32]
Rehearsal --------------------------------------------------------
core_rd_filelines_lf 1.076000 0.187000 1.263000 ( 1.263602)
----------------------------------------------- total: 1.263000sec

                       user     system      total        real

core_rd_filelines_lf 1.155000 0.125000 1.280000 ( 1.294803)

Updated by jonforums (Jon Forums) over 12 years ago

Still looking great! I'm working on getting you profiling info and test all results/diffs.

Results for text line reads on 500,000 LF delimited file. Your patch is the wio-ruby 1.9.3p0 results which beats the legacy 1.8.7p352 results :)

C:\projects\measurements-git>pik run rci bench core_rd_filelines_lf
jruby 1.6.3 (ruby-1.8.7-p330) (2011-07-07 965162f) (Java HotSpot(TM) Client VM 1.7.0_01) [Windows 7-x86-java]
Rehearsal --------------------------------------------------------
core_rd_filelines_lf 3.120000 0.000000 3.120000 ( 3.105000)
----------------------------------------------- total: 3.120000sec

                       user     system      total        real

core_rd_filelines_lf 3.057000 0.000000 3.057000 ( 3.057000)

ruby 1.8.7 (2011-06-30 patchlevel 352) [i386-mingw32]
Rehearsal --------------------------------------------------------
core_rd_filelines_lf 1.747000 0.266000 2.013000 ( 2.028004)
----------------------------------------------- total: 2.013000sec

                       user     system      total        real

core_rd_filelines_lf 1.779000 0.187000 1.966000 ( 1.965603)

ruby 1.9.2p290 (2011-07-09) [i386-mingw32]
Rehearsal --------------------------------------------------------
core_rd_filelines_lf 17.128000 0.094000 17.222000 ( 17.222430)
---------------------------------------------- total: 17.222000sec

                       user     system      total        real

core_rd_filelines_lf 16.755000 0.124000 16.879000 ( 16.894830)

ruby 1.9.3p0 (2011-11-08 revision 33661) [i386-mingw32]
Rehearsal --------------------------------------------------------
core_rd_filelines_lf 17.799000 0.031000 17.830000 ( 17.846432)
---------------------------------------------- total: 17.830000sec

                       user     system      total        real

core_rd_filelines_lf 18.019000 0.015000 18.034000 ( 18.049231)

wio-ruby 1.9.3p0 (2011-11-08 revision 33661) [i386-mingw32]
Rehearsal --------------------------------------------------------
core_rd_filelines_lf 1.092000 0.140000 1.232000 ( 1.232402)
----------------------------------------------- total: 1.232000sec

                       user     system      total        real

core_rd_filelines_lf 1.076000 0.141000 1.217000 ( 1.216802)

ruby 2.0.0dev (2011-11-08 trunk 33664) [i386-mingw32]
Rehearsal --------------------------------------------------------
core_rd_filelines_lf 16.661000 0.032000 16.693000 ( 16.707629)
---------------------------------------------- total: 16.693000sec

                       user     system      total        real

core_rd_filelines_lf 16.552000 0.046000 16.598000 ( 16.614029)

And results when doing binary read on the same test file just to make sure binary mode didn't get broken ;)

C:\projects\measurements-git>pik run rci bench core_brd_filelines_lf
jruby 1.6.3 (ruby-1.8.7-p330) (2011-07-07 965162f) (Java HotSpot(TM) Client VM 1.7.0_01) [Windows 7-x86-java]
Rehearsal ---------------------------------------------------------
core_brd_filelines_lf 1.871000 0.000000 1.871000 ( 1.820000)
------------------------------------------------ total: 1.871000sec

                        user     system      total        real

core_brd_filelines_lf 1.749000 0.000000 1.749000 ( 1.750000)

ruby 1.8.7 (2011-06-30 patchlevel 352) [i386-mingw32]
Rehearsal ---------------------------------------------------------
core_brd_filelines_lf 1.716000 0.218000 1.934000 ( 1.926110)
------------------------------------------------ total: 1.934000sec

                        user     system      total        real

core_brd_filelines_lf 1.700000 0.203000 1.903000 ( 1.910110)

ruby 1.9.2p290 (2011-07-09) [i386-mingw32]
Rehearsal ---------------------------------------------------------
core_brd_filelines_lf 0.858000 0.125000 0.983000 ( 1.008057)
------------------------------------------------ total: 0.983000sec

                        user     system      total        real

core_brd_filelines_lf 0.827000 0.172000 0.999000 ( 1.006058)

ruby 1.9.3p0 (2011-11-08 revision 33661) [i386-mingw32]
Rehearsal ---------------------------------------------------------
core_brd_filelines_lf 0.827000 0.156000 0.983000 ( 1.002058)
------------------------------------------------ total: 0.983000sec

                        user     system      total        real

core_brd_filelines_lf 0.858000 0.156000 1.014000 ( 1.029059)

wio-ruby 1.9.3p0 (2011-11-08 revision 33661) [i386-mingw32]
Rehearsal ---------------------------------------------------------
core_brd_filelines_lf 0.858000 0.125000 0.983000 ( 0.989057)
------------------------------------------------ total: 0.983000sec

                        user     system      total        real

core_brd_filelines_lf 0.796000 0.172000 0.968000 ( 0.966055)

ruby 2.0.0dev (2011-11-08 trunk 33664) [i386-mingw32]
Rehearsal ---------------------------------------------------------
core_brd_filelines_lf 0.826000 0.188000 1.014000 ( 1.022058)
------------------------------------------------ total: 1.014000sec

                        user     system      total        real

core_brd_filelines_lf 0.827000 0.202000 1.029000 ( 1.020059)

Updated by jonforums (Jon Forums) over 12 years ago

I'm having a very difficult time getting you make test-all comparisons because of these types of failures

http://www.mediafire.com/imgbnc.php/aa869e92ba544fb74b7576986810b881e4fb826581f4ac448d02d50f7cf2ef366g.jpg

That said, here's a snippet from the end of a run on ruby_1_9_3 without the patch. I have 113 listed errors and my scrollback buffer got overrun.
...
113) Error:
test_s_codepage_changed(TestWin32OLE):
WIN32OLERuntimeError: (in OLE method write': ) OLE error code:800A0005 in <Unknown> <No Description> HRESULT error code:0x80020009 Exception occurred. c:/Users/Jon/Documents/RubyDev/ruby-git/test/win32ole/test_win32ole.rb:355:in method_missing'
c:/Users/Jon/Documents/RubyDev/ruby-git/test/win32ole/test_win32ole.rb:355:in `test_s_codepage_changed'

10190 tests, 1884140 assertions, 10 failures, 11 errors, 92 skips
make: *** [yes-test-all] Error 21

So how should we proceed? Should we agree on a subset of tests similar to make test-all TESTS='openssl fiddle psych'?

Updated by h.shirosaki (Hiroshi Shirosaki) over 12 years ago

Jon, thank you for your work.
I have same difficulty. I have two error dialogs while 'make test-all'. I have to click and close error dialog to continue tests.
We can know test case which stops test by 'make test-all TESTS="-v"'. TESTS="-v" shows current test.

I think inserting "skip" to problematic test case makes test-all easier.
It's like as below.

def test_read_nonblock_error
return if !have_nonblock?
skip "IO#read_nonblock is not supported on file/pipe." if /mswin|bccwin|mingw/ =~ RUBY_PLATFORM

I have less errors and failures with "trunk ruby" on Windows 7 running on VirtualBox than your result.

Updated by jonforums (Jon Forums) over 12 years ago

Thank you and I will try make test-all TESTS='-v'

From my perspective, our challenge is to prove the patch works well against the current tests while keeping the issue from stagnating. It's great that the patch passes make test and make test-all TESTS='openssl fiddle psych'. It would be even nicer if make test-all functioned properly on MinGW so we could run all the current tests. However, this may not be practical or required.

To continue making forward progress, should we split the concerns into two issues?

First, let's identify the minimal set of current tests that will persuade us that your patch is safe, as your patch has already proven itself to be a significant usage benefit for Windows 1.9 users. For example, if your patch passes make test && make test-all TESTS='zlib openssl fiddle psych' are you and other ruby-core committers OK with committing your patch to ruby_1_9_3 and trunk? If not, what other tests should be added to TESTS?

Second, let's create a separate feature request to address the longer term test-all challenges on Windows. I personally like your skip idea and think it could be refined a bit. For example, could the skip method be hoisted into test/runner.rb (or even a new test/test_helper.rb) to allow for easy integration of platform and MRI version guards into the existing test suite? Also, should the platform matching use RbConfig::CONFIG['host_os'] instead? Perhaps I'm just not aware of existing platform guard-like capability using TESTS or other tricks.

I'm hoping we can find a clean (meaning, easy to retrofit into existing tests and easy-to-use) solution similar to http://rubyspec.org/guards/ I also hope that people don't resuscitate the old tests vs. RubySpec discussion, but rather, keep the focus on finding a pragmatic solution that enhances the existing test infrastructure. I strongly believe that when building on Windows (whether MinGW or VC++) one should be able to run [n]make && [n]make test && [n]make test-all to help verify the correctness of a build.

The reason I'm advocating splitting out the testing issue is so we can keep the focus on testing the safeness of your patch and keep moving forward on getting your patch accepted.

How should we proceed?

Updated by h.shirosaki (Hiroshi Shirosaki) over 12 years ago

I'm not sure how to do with test-all. mingw ruby seems not to pass all tests.
Anyway I tried to make test-all.
If ruby-core committers like this result, I hope this patch merged to trunk.

I have modified a patch to trunk. Previous patch conflicts with trunk. I have resolved and added minor fixes.

make test-all result become better.

  • patched ruby on Windows 7 with mingw32 gcc 4.5.2. running on VirtualBox
    10348 tests, 1884715 assertions, 7 failures, 5 errors, 94 skips

ruby -v: ruby 2.0.0dev (2011-11-10 trunk 33699) [i386-mingw32]

I have skipped test_https_proxy_authentication by mistake. So 1 skip is added.
Ruby on Windows XP chashes on this test, but this test are passed on Windows 7.

  • trunk ruby
    10348 tests, 1884718 assertions, 7 failures, 3 errors, 93 skips

ruby -v: ruby 2.0.0dev (2011-11-10 trunk 33699) [i386-mingw32]

Differences from ruby without patch are plus 2 errors. 1 failure is a different test.

patched ruby has
98) Failure:
test_run_skip_verbose(TestMiniTestUnit) [c:/Users/hiroshi/work/ruby/test/minitest/test_minitest_unit

.rb:392]:
103) Error:
test_require_invalid_shared_object(TestRequire):
104) Error:
test_capture3_flip(TestOpen3):

However,

'make test-all TESTS="-v test_open3.rb"'
'make test-all TESTS="-v ruby/test_require.rb"'
'make test-all TESTS="-v ruby/minitest/test_minitest_unit.rb"'

are passed without failures and errors. That's strange.

trunk ruby has below failure, but ruby with this patch does't have below failure.
99) Failure:
test_textmode(TestArgf) [c:/Users/hiroshi/work/ruby/test/ruby/test_argf.rb:657]:
[ruby-core:39234].
<"1\n2\n3\n4\n5\n6\n"> expected but was
<"1\r\n2\r\n3\r\n4\r\n5\r\n6\r\n">.

I attached a patch and test-all result.
Ruby on Windows XP has much more failure and error. Ruby on windows 7 is better.
How do you think about this result?

Updated by luislavena (Luis Lavena) over 12 years ago

  • Category set to core
  • Assignee set to usa (Usaku NAKAMURA)

Hiroshi Shirosaki wrote:

I'm not sure how to do with test-all. mingw ruby seems not to pass all tests.

That is correct, if you add skip to the following two:

TestRequire#test_require_invalid_shared_object
TestRubyOptions#test_segv_test

make test-all will complete

Anyway I tried to make test-all.
If ruby-core committers like this result, I hope this patch merged to trunk.

I have modified a patch to trunk. Previous patch conflicts with trunk. I have resolved and added minor fixes.

make test-all result become better.

The following is the list of tests that fails (using RubyInstaller's base) trunk versus your patch:

trunk:
TestArgf#test_textmode
TestDir_M17N#test_filename_bytes_euc_jp
TestDir_M17N#test_filename_euc_jp
TestDir_M17N#test_filename_ext_euc_jp_and_int_utf_8
TestDir_M17N#test_filename_extutf8
TestDir_M17N#test_filename_extutf8_inteucjp_representable
TestDir_M17N#test_filename_extutf8_inteucjp_unrepresentable
TestDir_M17N#test_filename_utf8_raw_name
TestGDBM#test_reorganize
TestGDBM#test_s_open_create_new
TestGDBM#test_s_open_error
TestGemInstaller#test_generate_bin_bindir_with_user_install_warning
TestRDocMarkupPreProcess#test_include_file
TestRDocMarkupPreProcess#test_include_file_encoding_incompatible
TestRubyOptions#test_encoding
TestUnicodeEscape#test_basic
TestWEBrickCGI#test_cgi
TestWin32OLE#test_s_codepage_changed

Versus your patch:
TestDir_M17N#test_filename_bytes_euc_jp
TestDir_M17N#test_filename_euc_jp
TestDir_M17N#test_filename_ext_euc_jp_and_int_utf_8
TestDir_M17N#test_filename_extutf8
TestDir_M17N#test_filename_extutf8_inteucjp_representable
TestDir_M17N#test_filename_extutf8_inteucjp_unrepresentable
TestDir_M17N#test_filename_utf8_raw_name
TestGDBM#test_reorganize
TestGDBM#test_s_open_create_new
TestGDBM#test_s_open_error
TestGemInstaller#test_generate_bin_bindir_with_user_install_warning
TestOpen3#test_capture3_flip
TestRDocMarkupPreProcess#test_include_file
TestRDocMarkupPreProcess#test_include_file_encoding_incompatible
TestRubyOptions#test_encoding
TestUnicodeEscape#test_basic
TestWEBrickCGI#test_cgi
TestWin32OLE#test_s_codepage_changed

===

In your patch TestArgf#test_textmode no longer fails, but TestOpen3#test_capture3_flip does.

trunk:

10347 tests, 1884751 assertions, 9 failures, 9 errors, 95 skips

your patch:

10347 tests, 1884748 assertions, 8 failures, 10 errors, 95 skips

===

It is worth to mention that write performance using your test script did improve:

V:>ruby -v t.rb
ruby 1.8.7 (2011-06-30 patchlevel 352) [i386-mingw32]
"0 0.598029"
"1 0.300001"
"2 0.2"

ruby 1.9.2p290 (2011-07-09) [i386-mingw32]
"0 0.413023"
"1 0.746043"
"2 0.935052"

ruby 1.9.3p0 (2011-10-30) [i386-mingw32]
"0 0.400014"
"1 0.721023"
"2 0.918049"

V:>ruby -v t.rb
ruby 2.0.0dev (2011-11-10 trunk 33700) [i386-mingw32]
"0 0.396023"
"1 0.725817"
"2 0.936044"

with patch
V:>ruby -v t.rb
ruby 2.0.0dev (2011-11-10 trunk 33700) [i386-mingw32]
"0 0.401023"
"1 0.238013"
"2 0.139008"

I'm assigning this to Usaku Nakamura for comments.

Thank you.

Updated by usa (Usaku NAKAMURA) over 12 years ago

  • Status changed from Open to Assigned

Since this patch should repeal the existing I/O canceling support mechanism,
I am anxious about influence unexpected very broadly coming out.
However, it seems that such big influence has not come out as long as seeing your result.

BTW, did you check "make test"?

especially, bootstaptest/test_io.rb

Updated by h.shirosaki (Hiroshi Shirosaki) over 12 years ago

BTW, did you check "make test"?

especially, bootstaptest/test_io.rb

I checked "make test". It passed with no fails.
Although I don't understand I/O canceling support mechanism yet, is there a way to overcome that?

Updated by usa (Usaku NAKAMURA) over 12 years ago

I checked "make test". It passed with no fails.

Good.

Although I don't understand I/O canceling support mechanism yet, is there a way to overcome that?

About this point, I am neutrality.
I think that priority may be given there if everyone says that
performance is more important.

Updated by h.shirosaki (Hiroshi Shirosaki) over 12 years ago

I found that TestOpen3#test_capture3_flip error goes away by below hack.
I think this hack is not so dangerous because similar hack is already used at lib/tempfile.rb line 236.

By this change, make test-all result become same except for TestArgf#test_textmode.

--- a/lib/tempfile.rb
+++ b/lib/tempfile.rb
@@ -276,7 +276,11 @@ class Tempfile < DelegateClass(File)

     # keep this order for thread safeness
     if path
  •      File.unlink(path) if File.exist?(path)
    
  •      begin
    
  •        File.unlink(path) if File.exist?(path)
    
  •      rescue Errno::EACCES
    
  •        # may not be able to unlink on Windows; just ignore
    
  •     end
       end
    
       STDERR.print "done\n" if $DEBUG
    

Updated by jonforums (Jon Forums) over 12 years ago

@usa (Usaku NAKAMURA), @luis (Luis Lopez),

What specific tasks must be completed before this patch will be accepted and committed to trunk?

Updated by luislavena (Luis Lavena) over 12 years ago

Usaku NAKAMURA wrote:

I think that priority may be given there if everyone says that
performance is more important.

Usa,

With latest test patch, do you think this is good to be merged?

Updated by h.shirosaki (Hiroshi Shirosaki) over 12 years ago

At TestOpen3#test_capture3_flip error, ERROR_SHARING_VIOLATION occurs because ruby tries to unlink a temp file while new process by capture3 are running. New process seems to have reference to the temp file.

In addition, some other temp files are left undeleted after make test-all is finished.
Would it be possible to delete temp files at another time such as at_exit?
This patch is an idea.

diff --git a/lib/tempfile.rb b/lib/tempfile.rb
index 22401b0..9344b5f 100644
--- a/lib/tempfile.rb
+++ b/lib/tempfile.rb
@@ -80,6 +80,13 @@ require 'thread'

mutex.

class Tempfile < DelegateClass(File)
MAX_TRY = 10 # :nodoc:

  • UNDELETED_FILES = []

  • Try to unlink undeleted temp files on Windows

  • at_exit do

  • UNDELETED_FILES.each do |path|

  •  (File.unlink(path) if File.exist?(path)) rescue nil
    
  • end

  • end
    include Dir::Tmpname

    call-seq:

@@ -235,6 +242,7 @@ class Tempfile < DelegateClass(File)
@tmpname = nil
rescue Errno::EACCES
# may not be able to unlink on Windows; just ignore

  •  UNDELETED_FILES << path
    

    end
    end
    alias delete unlink
    @@ -276,7 +284,12 @@ class Tempfile < DelegateClass(File)

       # keep this order for thread safeness
       if path
    
  •      File.unlink(path) if File.exist?(path)
    
  •      begin
    
  •        File.unlink(path) if File.exist?(path)
    
  •      rescue Errno::EACCES
    
  •        # may not be able to unlink on Windows; just ignore
    
  •        UNDELETED_FILES << path
    
  •      end
       end
    
       STDERR.print "done\n" if $DEBUG
    

Updated by akr (Akira Tanaka) over 12 years ago

2011/11/18 Hiroshi Shirosaki :

Issue #5562 has been updated by Hiroshi Shirosaki.

At TestOpen3#test_capture3_flip error, ERROR_SHARING_VIOLATION occurs because ruby tries to unlink a temp file while new process by capture3 are running. New process seems to have reference to the temp file.

test_capture3_flip doesn't use Tempfile.
Why a temp file is referenced by the process created by test_capture3_flip?

Tanaka Akira

Updated by usa (Usaku NAKAMURA) over 12 years ago

Hello,

In message "[ruby-core:41124] [ruby-trunk - Feature #5562] Improvement of Windows IO performance"
on Nov.18,2011 11:56:44, wrote:

At TestOpen3#test_capture3_flip error, ERROR_SHARING_VIOLATION occurs because ruby tries to unlink a temp file while new process by capture3 are running. New process seems to have reference to the temp file.

Hmm, does r33783 help you?

Regards,

U.Nakamura

Updated by h.shirosaki (Hiroshi Shirosaki) over 12 years ago

test_capture3_flip doesn't use Tempfile.
Why a temp file is referenced by the process created by test_capture3_flip?

Minimal test case is as below.

make test-all TESTS="net/http/test_http.rb test_open3.rb"

test_http.rb uses tempfiles.
file = Tempfile.new('ruby-test')

All temp files are not deleted when test_http.rb is finished.

When test_open3.rb is executed, ruby gc triggers tempfile finalizer and causes ERROR_SHARING_VIOLATION.

Updated by h.shirosaki (Hiroshi Shirosaki) over 12 years ago

test_capture3_flip doesn't use Tempfile.
Why a temp file is referenced by the process created by test_capture3_flip?

Minimal test case is as below.

make test-all TESTS="net/http/test_http.rb test_open3.rb"

test_http.rb uses tempfiles.
file = Tempfile.new('ruby-test')

All temp files are not deleted when test_http.rb is finished.

When test_open3.rb is executed, ruby gc triggers tempfile finalizer and causes ERROR_SHARING_VIOLATION.

Updated by h.shirosaki (Hiroshi Shirosaki) over 12 years ago

Hmm, does r33783 help you?

Thank you.
I merged r33783 and checked make test-all TESTS="net/http/test_http.rb test_open3.rb".
It passed without error. That fix works.

I found a document.
http://msdn.microsoft.com/en-us/library/z0kc8e3z(v=VS.90).aspx
_O_NOINHERIT
Prevents creation of a shared file descriptor.

Updated by h.shirosaki (Hiroshi Shirosaki) over 12 years ago

In addition, some other temp files are left undeleted after make test-all is finished.

About above point, some temp files are still left after applying r33783.
This would be for another reason. Undeleted temp files are as below.

d20111119-2768-14ggack
d20111119-2768-1yhmqea
must_ac_20111119-2768-7vc6cv
test_csv.20111119-2768-1nzjjmo.csv
test_csv.20111119-2768-1wb3fvw.csv
test_csv.20111119-2768-eb3km3.csv
test_csv.20111119-2768-z2pbn5.csv
yikes20111119-2768-88ag4oyml

Updated by luislavena (Luis Lavena) over 12 years ago

Hello,

Bumping this.

Hiroshi Shirosaki: can you upload a newer patch against latest trunk?

Usaku NAKAMURA: Can I apply Hiroshi's patch to trunk once is updated?

Thank you.

Updated by usa (Usaku NAKAMURA) over 12 years ago

Hello,

In message "[ruby-core:41331] [ruby-trunk - Feature #5562] Improvement of Windows IO performance"
on Nov.27,2011 23:49:02, wrote:

Usaku NAKAMURA: Can I apply Hiroshi's patch to trunk once is updated?

Yes. It's acceptable supposing nobody opposes.

Regards,

U.Nakamura

Updated by h.shirosaki (Hiroshi Shirosaki) over 12 years ago

Hiroshi Shirosaki: can you upload a newer patch against latest trunk?

Yes, I'll update a patch. It takes some time for check and test.

Thank you.

Hiroshi Shirosaki

Updated by h.shirosaki (Hiroshi Shirosaki) over 12 years ago

I updated a patch against trunk@33908.

Patched ruby passed "make test" with no failures.

Here are test-all results.

patched ruby

OpenSSL::TestConfig#test_constants = 0.05 s = F
TestDir_M17N#test_filename_as_bytes_extutf8 = 0.45 s = E
TestDir_M17N#test_filename_extutf8_inteucjp_unrepresentable = 0.72 s = E
TestGDBM#test_reorganize = 0.12 s = F
TestGDBM#test_s_open_create_new = 0.11 s = F
TestGDBM#test_s_open_error = 0.08 s = F
TestGemInstaller#test_generate_bin_bindir_with_user_install_warning = 0.57 s = F
TestModule#test_ancestors = 0.00 s = F
TestModule#test_included_modules = 0.02 s = F

10300 tests, 1921758 assertions, 7 failures, 2 errors, 95 skips

ruby -v: ruby 2.0.0dev (2011-11-30 trunk 33908) [i386-mingw32]

trunk ruby

OpenSSL::TestConfig#test_constants = 0.02 s = F
TestDir_M17N#test_filename_as_bytes_extutf8 = 0.42 s = E
TestDir_M17N#test_filename_extutf8_inteucjp_unrepresentable = 0.67 s = E
TestGDBM#test_reorganize = 0.11 s = F
TestGDBM#test_s_open_create_new = 0.06 s = F
TestGDBM#test_s_open_error = 0.06 s = F
TestGemInstaller#test_generate_bin_bindir_with_user_install_warning = 0.42 s = F
TestIO_M17N#test_s_pipe_undef = 0.06 s = E
TestModule#test_ancestors = 0.00 s = F
TestModule#test_included_modules = 0.00 s = F

10300 tests, 1921739 assertions, 7 failures, 3 errors, 95 skips

ruby -v: ruby 2.0.0dev (2011-11-30 trunk 33908) [i386-mingw32]

I added two tests for binmode method. This patch contains two tests.
I preserved original newline conversion behavior of stdin and pipe read for binmode works properly.
So stdin and pipe read remain slow. I assume these performance wouldn't be so important.
I confirmed patched ruby's file write and read still faster.

Trunk ruby has one more error, but make test-all TESTS="ruby/test_io_m17n.rb" passed with no errors.

  1. Error:
    test_s_pipe_undef(TestIO_M17N):
    Errno::EBADF: Bad file descriptor
    c:/Users/hiroshi/work/ruby_trunk/test/ruby/test_io_m17n.rb:309:in read' c:/Users/hiroshi/work/ruby_trunk/test/ruby/test_io_m17n.rb:309:in block in test_s_pipe_undef'
    c:/Users/hiroshi/work/ruby_trunk/test/ruby/test_io_m17n.rb:27:in call' c:/Users/hiroshi/work/ruby_trunk/test/ruby/test_io_m17n.rb:27:in block in pipe'
    c:/Users/hiroshi/work/rubyinstaller/sandbox/ruby19_build/.ext/common/win32ole.rb:13:in call' c:/Users/hiroshi/work/rubyinstaller/sandbox/ruby19_build/.ext/common/win32ole.rb:13:in block in initialize'

About OpenSSL failure, below "C:/projects/rubyinstaller-git" directory doesn't exist in my environment.
Is this rubyinstaller's issue?

  1. Failure:
    test_constants(OpenSSL::TestConfig) [c:/Users/hiroshi/work/ruby/test/openssl/test_config.rb:20]:
    Exception raised:
    <#<Errno::ENOENT: No such file or directory - C:/projects/rubyinstaller-git/sandbox/openssl/ssl/openssl.cnf>>.

I checked patched ruby on Mac OSX.

Build succeeded.
"make test" passed.
"make test-all" had below two failures.

TestModule#test_ancestors
TestModule#test_included_modules

Please evaluate this patch.
Thank you for your consideration.

Updated by h.shirosaki (Hiroshi Shirosaki) over 12 years ago

Additionally, I used Luis's patch of #5681 for build.

I skipped the following two test.
TestRequire#test_require_invalid_shared_object
TestRubyOptions#test_segv_test

Updated by luislavena (Luis Lavena) over 12 years ago

  • Assignee changed from usa (Usaku NAKAMURA) to luislavena (Luis Lavena)
Actions #39

Updated by luislavena (Luis Lavena) over 12 years ago

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

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


Introduce NEED_READCONV and NEED_WRITECONV to replace universal newline decorator

Use CRLF only when required to improve file reading and writing under Windows.
Patch by Hiroshi Shirosaki. [ruby-core:40706] [Feature #5562]

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0