[Jython-bugs] [ jython-Patches-1854873 ] Patch for [ 1841644 ] Mmap module is not in jython

[SourceForge.net <no...@so...>]

Patches item #1854873, was opened at 2007-12-20 08:36
Message generated for change (Comment added) made by cgroves
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=312867&aid=1854873&group_id=12867

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Mehendran (mehendran)
Assigned to: Nobody/Anonymous (nobody)
Summary: Patch for [ 1841644 ] Mmap module is not in jython

Initial Comment:
I have uploaded the patch for mmap module. 
Please send your valuable comments.
Thanks,
Mehendran

----------------------------------------------------------------------

>Comment By: Charles Groves (cgroves)
Date: 2007-12-24 01:09

Message:
Logged In: YES 
user_id=1174327
Originator: NO

The javadocs for FileChannel.map say that the mapped buffer isn't tied to
the channel that created it, so that even if the channel is closed, the
buffer is still available.  Why not just require that the fileno that comes
in be an instance of FileChannel, since that's what we return for fileno in
instances of files?  Changing the type of object passed into mmap.mmap
makes this module different enough from the CPython version that sharing a
name with it is probably more harmful than good.  Definitely any CPython
code that uses mmap will have to be special cased to work with Jython.

I don't understand why PyMmap needs to extend PyString.  The mmap type in
CPython is not an instance of str.  However in Jython, extending PyString
and keeping a String copy of the mmapped data breaks the general use case
of mmapping.  From my understanding, when you mmap a file you're expecting
the OS to map it into memory as you're reading it, and then to page it out
as parts of it are no longer being read.  Since this patch reads the whole
file into a Java String, it's actually far worse than reading the file into
a single byte array and holding onto it: it'll use twice the memory of the
file for chars instead of bytes.  This difference is going to make mmap
useless for anyone mapping a large file.  Where all is it expected to be an
actual instance of str?  I see re.search checking that the pattern is a
str, but not the object to be searched.

What do you mean "a doubt about implementing the buffer interface"?  I'm
not sure what the buffer interface is.

Your check for os.name starting with "Lin" as a check for unix is a pretty
bad idea since there are several unix variants out there that have a JVM
that don't start with "Lin".  Is there any reason to actually support an os
distinction on Jython's version of mmap?  Since it's the same across all
platforms, why not just use the access keyword and raise errors for flag,
prot and tagname?  Platform independent mmapping has to use access
already.

I'd really like to avoid using internal Sun packages in Jython if at all
possible: if we use them, we can't run on all JVMs.  Is there any reason
for the close method to actually "close" the mmaped buffer?  Why not just
null out the reference to it and let the garbage collector get it when it
will?  Then you can just check for the buffer being null on any operation
on the mmap type and raise an exception if it happens.

These are the major issues I noticed in a quick check of the patch.  There
may be smaller issues turned up by a closer examination, but I figured it
was a good idea to get these out of the way before doing that.

----------------------------------------------------------------------

Comment By: Mehendran (mehendran)
Date: 2007-12-20 08:42

Message:
Logged In: YES 
user_id=1831942
Originator: YES

I have attached a doc that explains about issues, differences
and implementation details of mmap module.
Suggestions and Comments are highly expected. 
File Added: mmap_doc.txt

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=312867&aid=1854873&group_id=12867