Skip to content

Commit

Permalink
fix(MapWindow): unicode foes in read_into_memory() used by gitpython TCs
Browse files Browse the repository at this point in the history
Drop Windows only codepath bypassing memory-mapping due to some leaks in
the past. 
Now Appveyor proves everything run ok.  
Additionally, this codepath had unicode problems on PY3. So deleting it,
fixes 2 TCs in gitpython:
+ TestRepo.test_file_handle_leaks()
+ TestObjDbPerformance.test_random_access()

See gitpython-developers/GitPython#525
  • Loading branch information
ankostis committed Oct 22, 2016
1 parent ac5df70 commit b2eafcc
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 28 deletions.
16 changes: 10 additions & 6 deletions smmap/mman.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ class WindowCursor(object):
__slots__ = (
'_manager', # the manger keeping all file regions
'_rlist', # a regions list with regions for our file
'_region', # our current region or None
'_ofs', # relative offset from the actually mapped area to our start area
'_size' # maximum size we should provide
'_region', # our current class:`MapRegion` or None
'_ofs', # relative offset from the actually mapped area to our start area
'_size' # maximum size we should provide
)

def __init__(self, manager=None, regions=None):
Expand Down Expand Up @@ -308,10 +308,14 @@ def _collect_lru_region(self, size):
if 0, we try to free any available region
:return: Amount of freed regions
**Note:** We don't raise exceptions anymore, in order to keep the system working, allowing temporary overallocation.
If the system runs out of memory, it will tell.
.. Note::
We don't raise exceptions anymore, in order to keep the system working, allowing temporary overallocation.
If the system runs out of memory, it will tell.
**todo:** implement a case where all unusued regions are discarded efficiently. Currently its only brute force"""
.. TODO::
implement a case where all unusued regions are discarded efficiently.
Currently its only brute force
"""
num_found = 0
while (size == 0) or (self._memory_size + size > self._max_memory_size):
lru_region = None
Expand Down
25 changes: 3 additions & 22 deletions smmap/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,16 +116,13 @@ class MapRegion(object):
'_size', # cached size of our memory map
'__weakref__'
]
_need_compat_layer = sys.version_info[0] < 3 and sys.version_info[1] < 6
_need_compat_layer = sys.version_info[:2] < (2, 6)

if _need_compat_layer:
__slots__.append('_mfb') # mapped memory buffer to provide offset
# END handle additional slot

#{ Configuration
# Used for testing only. If True, all data will be loaded into memory at once.
# This makes sure no file handles will remain open.
_test_read_into_memory = False
#} END configuration

def __init__(self, path_or_fd, ofs, size, flags=0):
Expand Down Expand Up @@ -160,10 +157,7 @@ def __init__(self, path_or_fd, ofs, size, flags=0):
# bark that the size is too large ... many extra file accesses because
# if this ... argh !
actual_size = min(os.fstat(fd).st_size - sizeofs, corrected_size)
if self._test_read_into_memory:
self._mf = self._read_into_memory(fd, ofs, actual_size)
else:
self._mf = mmap(fd, actual_size, **kwargs)
self._mf = mmap(fd, actual_size, **kwargs)
# END handle memory mode

self._size = len(self._mf)
Expand All @@ -179,19 +173,6 @@ def __init__(self, path_or_fd, ofs, size, flags=0):
# We assume the first one to use us keeps us around
self.increment_client_count()

def _read_into_memory(self, fd, offset, size):
""":return: string data as read from the given file descriptor, offset and size """
os.lseek(fd, offset, os.SEEK_SET)
mf = ''
bytes_todo = size
while bytes_todo:
chunk = 1024 * 1024
d = os.read(fd, chunk)
bytes_todo -= len(d)
mf += d
# END loop copy items
return mf

def __repr__(self):
return "MapRegion<%i, %i>" % (self._b, self.size())

Expand Down Expand Up @@ -267,7 +248,7 @@ class MapRegionList(list):
"""List of MapRegion instances associating a path with a list of regions."""
__slots__ = (
'_path_or_fd', # path or file descriptor which is mapped by all our regions
'_file_size' # total size of the file we map
'_file_size' # total size of the file we map
)

def __new__(cls, path):
Expand Down

0 comments on commit b2eafcc

Please sign in to comment.