Project

General

Profile

Actions

Backport #5704

closed

Please backport r33937 (newline decorator)

Added by luislavena (Luis Lavena) over 10 years ago. Updated over 10 years ago.

Status:
Closed
Priority:
Normal
[ruby-core:41468]

Description

Hello,

r33937 contains what was labeled 'feature' for ruby-trunk (Issue #5562), however, it was considered a bug and acknowledged by developers in #1332

Quoting Yusuke Endoh:

"Yes, text mode is still 10x -- 30x slower than binary mode.
It is reproduced not only on windows but also Linux.
Perhaps, this is the symptom because of the reason explained
in [ruby-core:26515]."

And quoting Usaku Nakamura:

"Currently, we has implemented the newline conversion as a
transcode converter, just like encoding conversion.
But the design of transcode is too general to use it such
a simple operation, as our finding.
We want to find a better mechanism which doesn't deviate
from the current design of IO..."

The patch provided by Hiroshi Shirosaki speeds up both file read and write operations under Windows to decent levels while do not deviate from current IO design.

Please consider this for backporting as we see it as Bug and not Feature.


Files

commitlog.txt (4.06 KB) commitlog.txt h.shirosaki (Hiroshi Shirosaki), 02/04/2012 08:52 PM
winio_backport.patch (38.9 KB) winio_backport.patch h.shirosaki (Hiroshi Shirosaki), 02/04/2012 08:52 PM

Related issues 1 (0 open1 closed)

Related to Ruby master - Feature #5562: Improvement of Windows IO performanceClosedluislavena (Luis Lavena)11/03/2011Actions

Updated by jonforums (Jon Forums) over 10 years ago

FYI, in the hope that it helps with backporting, a branch containing Hiroshi's patch is being maintained at

https://github.com/thecodeshop/ruby/commits/winio/ruby_1_9_3/

The branch also contains backports of r33200 and r33662 so that the following tests pass when building with the RubyInstaller build recipes on Win7 32bit SP1:

make test-all TESTS='openssl fiddle psych zlib io ruby/test_io.rb ruby/test_io_m17n.rb ruby/test_file.rb'
make test

Updated by jonforums (Jon Forums) over 10 years ago

Please ensure the r33944 fixup patch for this issue is also backported.

Updated by kosaki (Motohiro KOSAKI) over 10 years ago

  • Subject changed from Please backport r33937 to Please backport r33937 (newline decorator)

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

At [ruby-core:42102], Luis mentioned

I took the assignation of above tickets and will perform the backports
next week tops.

If another committer disagree, please say so.

It seems no one disagree.
So I'll do the backports. I heard Luis was very busy with other work.

Backport patches are maintained at
https://github.com/thecodeshop/ruby/commits/winio/ruby_1_9_3

These issues (11 commits) are a set of fixes. So I'll backport with one commit.

http://bugs.ruby-lang.org/issues/5794
http://bugs.ruby-lang.org/issues/5793
http://bugs.ruby-lang.org/issues/5792
http://bugs.ruby-lang.org/issues/5791

I attached a backport patch and commit log.
I'll commit that with version.h PATCHLEVEL fix.

-#define RUBY_PATCHLEVEL 31
+#define RUBY_PATCHLEVEL 32

I confirmed make, make test and make test-all are no problem.

Is this OK?

I cannot change redmine issue's status. Could anyone register me as a developer on redmine?

Updated by usa (Usaku NAKAMURA) over 10 years ago

  • Status changed from Open to Assigned
  • Assignee changed from yugui (Yuki Sonoda) to usa (Usaku NAKAMURA)
Actions #6

Updated by naruse (Yui NARUSE) over 10 years ago

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

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


merge revision(s) 33937: [Backport #5704]

* ext/zlib/zlib.c (rb_gzreader_initialize): use binary mode by default
  under Windows. Patch by Hiroshi Shirosaki. [ruby-core:40706]
  [Feature #5562]

* include/ruby/encoding.h (void rb_econv_binmode): define NEWLINE
  decorator.

* io.c (rb_cloexec_fcntl_dupfd): Introduce NEED_READCONV and
  NEED_WRITECONV to replace universal newline decorator by CRLF only
  when required to improve file reading and writing under Windows.
  Patch by Hiroshi Shirosaki. [ruby-core:40706] [Feature #5562]

* io.c (do_writeconv): adjust binary mode if required.

* io.c (read_all, appendline, swallow, rb_io_getline_1): ditto.

* io.c (io_getc, rb_io_each_codepoint, rb_io_ungetc): ditto.

* io.c (rb_io_binmode, rb_io_ascii8bit_binmode): ditto.

* io.c (rb_io_extract_modeenc, rb_sysopen): ditto.

* io.c (pipe_open, prep_stdio, io_encoding_set): ditto.

* io.c (rb_io_s_pipe, copy_stream_body): ditto.

* test/ruby/test_io_m17n.rb (EOT): add test for pipe and stdin in
  binary mode.

* win32/win32.c (init_stdhandle): remove O_BINARY from stdhandle
  initialization.

* win32/win32.c (rb_w32_write): use FTEXT mode accordingly.
Actions

Also available in: Atom PDF