Feature #4532
closed[PATCH] add IO#pread and IO#pwrite methods
Added by normalperson (Eric Wong) over 13 years ago. Updated over 7 years ago.
Description
=begin
These methods are useful for safe/concurrent file I/O in
multi-thread/process environments and also fairly standard
nowadays especially in systems supporting pthreads.
pread() is already used internally for IO.copy_stream
=end
Files
0001-add-IO-pread-and-IO-pwrite-methods.patch (6.22 KB) 0001-add-IO-pread-and-IO-pwrite-methods.patch | patch with tests to implement feature | normalperson (Eric Wong), 03/28/2011 02:06 PM | |
0001-Add-IO-pread-and-IO-pwrite-methods.patch (6.8 KB) 0001-Add-IO-pread-and-IO-pwrite-methods.patch | Updated patch, for current trunk (57443) | avsej (Sergey Avseyev), 01/27/2017 08:29 PM | |
0001-Add-IO-pread-and-IO-pwrite-methods-v3.patch (6.98 KB) 0001-Add-IO-pread-and-IO-pwrite-methods-v3.patch | avsej (Sergey Avseyev), 01/27/2017 08:52 PM |
Updated by kosaki (Motohiro KOSAKI) over 13 years ago
2011/3/28 Eric Wong normalperson@yhbt.net:
Issue #4532 has been reported by Eric Wong.
Feature #4532: [PATCH] add IO#pread and IO#pwrite methods
http://redmine.ruby-lang.org/issues/4532Author: Eric Wong
Status: Open
Priority: Normal
Assignee:
Category: core
Target version: 1.9.xThese methods are useful for safe/concurrent file I/O in
multi-thread/process environments and also fairly standard
nowadays especially in systems supporting pthreads.pread() is already used internally for IO.copy_stream
Do we really need to introduce new method? Why can't we overload
IO.read and IO.write?
too complex?
I agree "offset" argument is useful. But I'm not convinced this API
design is best. The description
is too quiet.
Ok, back to meta reviewing comments. All new API proposal need to
explain why this way
is best and need to persuade matz.
Updated by normalperson (Eric Wong) over 13 years ago
KOSAKI Motohiro kosaki.motohiro@gmail.com wrote:
2011/3/28 Eric Wong normalperson@yhbt.net:
Issue #4532 has been reported by Eric Wong.
Feature #4532: [PATCH] add IO#pread and IO#pwrite methods
http://redmine.ruby-lang.org/issues/4532Author: Eric Wong
Status: Open
Priority: Normal
Assignee:
Category: core
Target version: 1.9.xThese methods are useful for safe/concurrent file I/O in
multi-thread/process environments and also fairly standard
nowadays especially in systems supporting pthreads.pread() is already used internally for IO.copy_stream
Do we really need to introduce new method? Why can't we overload
IO.read and IO.write?
too complex?
IO#read and IO#write take userspace buffers into account which
makes no sense with pread/pwrite.
I considered overloading IO#sysread and IO#syswrite, but it would be
hard for users to determine whether offset is supported on their
platform.
New methods means IO.method_defined? and IO#respond_to? to be used.
I'm not a fan of throwing NotImplementedError and faking with lseek() +
read()/write() to be even worse since it loses the atomicity guarantee.
I also considered putting the new methods in File instead of IO, but
sysseek is an IO method so I put it in IO.
I agree "offset" argument is useful. But I'm not convinced this API
design is best. The description
is too quiet.Ok, back to meta reviewing comments. All new API proposal need to
explain why this way
is best and need to persuade matz.
Thanks again for your time!
--
Eric Wong
Updated by kosaki (Motohiro KOSAKI) over 13 years ago
2011/3/29 Eric Wong normalperson@yhbt.net:
KOSAKI Motohiro kosaki.motohiro@gmail.com wrote:
2011/3/28 Eric Wong normalperson@yhbt.net:
Issue #4532 has been reported by Eric Wong.
Feature #4532: [PATCH] add IO#pread and IO#pwrite methods
http://redmine.ruby-lang.org/issues/4532Author: Eric Wong
Status: Open
Priority: Normal
Assignee:
Category: core
Target version: 1.9.xThese methods are useful for safe/concurrent file I/O in
multi-thread/process environments and also fairly standard
nowadays especially in systems supporting pthreads.pread() is already used internally for IO.copy_stream
Do we really need to introduce new method? Why can't we overload
IO.read and IO.write?
too complex?IO#read and IO#write take userspace buffers into account which
makes no sense with pread/pwrite.
userspace buffer is implementation detail. no?
And, If pread is always behave as binary mode read method, your
documentation is much misleading. IMHO.
-
f = File.new("testfile")
-
f.pread(16, 0) #=> "This is line one"
I considered overloading IO#sysread and IO#syswrite, but it would be
hard for users to determine whether offset is supported on their
platform.
?? Why?
Do you have any example?
New methods means IO.method_defined? and IO#respond_to? to be used.
OK, fair point.
I'm not a fan of throwing NotImplementedError and faking with lseek() +
read()/write() to be even worse since it loses the atomicity guarantee.
I disagree. I dislike following part of your patch.
#ifdef HAVE_PREAD
rb_define_method(rb_cIO, "pread", rb_io_pread, -1);
#endif
This is very wrong style for new method. Eventually, all users need to call
method_defined? before pread. Just NotImplementedError (or lseek emulation)
makes much simpler script.
I also considered putting the new methods in File instead of IO, but
sysseek is an IO method so I put it in IO.
I agree IO is better.
Updated by normalperson (Eric Wong) over 13 years ago
KOSAKI Motohiro kosaki.motohiro@gmail.com wrote:
2011/3/29 Eric Wong normalperson@yhbt.net:
KOSAKI Motohiro kosaki.motohiro@gmail.com wrote:
Do we really need to introduce new method? Why can't we overload
IO.read and IO.write?
too complex?IO#read and IO#write take userspace buffers into account which
makes no sense with pread/pwrite.userspace buffer is implementation detail. no?
It's a very important detail. Different processes don't share userspace
buffers and Ruby should be able to share buffers with others (non-Ruby
processes), so we must only use the kernel cache.
And, If pread is always behave as binary mode read method, your
documentation is much misleading. IMHO.
f = File.new("testfile")
f.pread(16, 0) #=> "This is line one"
Yes, I just copied docs from sysread/syswrite :x I will update them
in the next patch I post.
I'm not a fan of throwing NotImplementedError and faking with lseek() +
read()/write() to be even worse since it loses the atomicity guarantee.I disagree. I dislike following part of your patch.
#ifdef HAVE_PREAD
rb_define_method(rb_cIO, "pread", rb_io_pread, -1);
#endifThis is very wrong style for new method. Eventually, all users need to call
method_defined? before pread. Just NotImplementedError (or lseek emulation)
makes much simpler script.
OK, shall I resubmit with adding optional offset argument to
sysread/syswrite and raise NotImplementedError?
Thank you again for your comments!
--
Eric Wong
Updated by kosaki (Motohiro KOSAKI) over 13 years ago
- Status changed from Open to Assigned
- Assignee set to matz (Yukihiro Matsumoto)
Hi
I personally still dislike pread/pwrite method name, but I give up to argue it because I'm worry about making endless discussion. Let's assign this ticket to matz and hear his opinions. :)
Updated by ko1 (Koichi Sasada) almost 12 years ago
- Description updated (diff)
- Assignee changed from matz (Yukihiro Matsumoto) to kosaki (Motohiro KOSAKI)
- Target version changed from 2.0.0 to 2.6
kosaki-san, could you talk with matz about this ticket?
Please change target to 2.0 if you success to persuade matz.
Updated by avsej (Sergey Avseyev) over 7 years ago
Hi everyone, any plans to include pread/pwrite in near future?
Updated by avsej (Sergey Avseyev) over 7 years ago
- File 0001-Add-IO-pread-and-IO-pwrite-methods.patch 0001-Add-IO-pread-and-IO-pwrite-methods.patch added
I rebased the patch against current trunk, and also made some improvements:
- raise NotImplementedError on platforms, which do not support pread/pwrite
- improved documentation
- fix argument order for IO#pwrite to be consistent with pwrite(2) and IO#pread, the offset should go last
- update tests and function names to follow the same style as other code
Updated by avsej (Sergey Avseyev) over 7 years ago
- File 0001-Add-IO-pread-and-IO-pwrite-methods-v3.patch added
The same patch as above, but with typo fixes
Updated by avsej (Sergey Avseyev) over 7 years ago
- File deleted (
0001-Add-IO-pread-and-IO-pwrite-methods-v3.patch)
Updated by matz (Yukihiro Matsumoto) over 7 years ago
- Assignee changed from kosaki (Motohiro KOSAKI) to nobu (Nobuyoshi Nakada)
Agreed. pread/pwrite can be useful in some cases.
Matz.
Updated by nobu (Nobuyoshi Nakada) over 7 years ago
- Status changed from Assigned to Closed
Applied in changeset trunk|r58240.
Add IO#pread and IO#pwrite methods
These methods are useful for safe/concurrent file I/O in
multi-thread/process environments and also fairly standard
nowadays especially in systems supporting pthreads.
Based on patches by Avseyev sergey.avseyev@gmail.com at
[ruby-core:79290]. [Feature #4532]
-
configure.in: check for pwrite(2). pread() is already used
internally for IO.copy_stream. -
io.c: implement wrappers for pread(2) and pwrite(2) and expose
them in IO.