Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix mmap_array with offset on Windows #7242

Merged
merged 3 commits into from
Jun 15, 2014
Merged

Fix mmap_array with offset on Windows #7242

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
63e9fdc
Fix mmap with offset on Windows
tkelman Jun 13, 2014
8d3faee
fix szfile and szarray values
tkelman Jun 14, 2014
712d81c
add extra error message info from Windows API
tkelman Jun 14, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions base/mmap.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,25 +127,29 @@ function mmap_array{T,N}(::Type{T}, dims::NTuple{N,Integer}, s::IO, offset::File
ro = isreadonly(s)
flprotect = ro ? 0x02 : 0x04
len = prod(dims)*sizeof(T)
const granularity::Int = ccall(:jl_getallocationgranularity, Clong, ())
if len < 0
error("requested size is negative")
end
if len > typemax(Int)
if len > typemax(Int)-granularity
error("file is too large to memory-map on this platform")
end
# Set the offset to a page boundary
offset_page::FileOffset = ifloor(offset/granularity)*granularity
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use idiv instead?

szarray = convert(Csize_t, len)
szfile = szarray + convert(Csize_t, offset)
szfile = szarray + convert(Csize_t, offset-offset_page)
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is szfile? Looks like it used to be a pointer to the last byte, and now is a length?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dwMaximumSize - quoting MSDN, "maximum size of the file mapping object"

You probably know more about this stuff than I do, I'm not 100% sure I did this right. Still some trouble with HDF5, and when I tried a large offset, but at least this was able to fix the simple case that's currently broken.

mmaphandle = ccall(:CreateFileMappingW, stdcall, Ptr{Void}, (Ptr{Void}, Ptr{Void}, Cint, Cint, Cint, Ptr{Uint16}),
shandle.handle, C_NULL, flprotect, szfile>>32, szfile&0xffffffff, C_NULL)
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from reading msdn, it appears that szfile here should = offset + szarray

or it can equal 0, without any actual cost (http://msdn.microsoft.com/en-us/library/windows/desktop/aa366542(v=vs.85).aspx)

if mmaphandle == C_NULL
error("could not create file mapping")
end
access = ro ? 4 : 2
viewhandle = ccall(:MapViewOfFile, stdcall, Ptr{Void}, (Ptr{Void}, Cint, Cint, Cint, Csize_t), mmaphandle, access, offset>>32, offset&0xffffffff, szarray)
viewhandle = ccall(:MapViewOfFile, stdcall, Ptr{Void}, (Ptr{Void}, Cint, Cint, Cint, Csize_t),
mmaphandle, access, offset_page>>32, offset_page&0xffffffff, szarray)
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from reading msdn, it appears that szarray here should be szarray+offset-offset_page

0xffff_ffff can also be written as typemax(Uint32) for clarity (or at least with an underscore)

if viewhandle == C_NULL
error("could not create mapping view")
end
A = pointer_to_array(pointer(T, viewhandle), dims)
A = pointer_to_array(pointer(T, viewhandle+offset-offset_page), dims)
finalizer(A, x->munmap(viewhandle, mmaphandle))
return A
end
Expand Down
1 change: 1 addition & 0 deletions src/julia.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -813,6 +813,7 @@ DLLEXPORT int32_t jl_stat(const char *path, char *statbuf);
DLLEXPORT void NORETURN jl_exit(int status);
DLLEXPORT int jl_cpu_cores(void);
DLLEXPORT long jl_getpagesize(void);
DLLEXPORT long jl_getallocationgranularity(void);
DLLEXPORT int jl_is_debugbuild(void);

// environment entries
Expand Down
18 changes: 18 additions & 0 deletions src/sys.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,24 @@ long jl_getpagesize(void)
}
#endif

#ifdef _OS_WINDOWS_
static long cachedAllocationGranularity = 0;
long jl_getallocationgranularity(void)
{
if (!cachedAllocationGranularity) {
SYSTEM_INFO systemInfo;
GetSystemInfo (&systemInfo);
cachedAllocationGranularity = systemInfo.dwAllocationGranularity;
}
return cachedAllocationGranularity;
}
#else
long jl_getallocationgranularity(void)
{
return jl_getpagesize();
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we just merge these two functions, since they are only used on their respective platforms?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern there was whether anyone currently uses the page size on Windows. Technically it would be a breaking change to start giving a different answer.

}
#endif

DLLEXPORT long jl_SC_CLK_TCK(void)
{
#ifndef _OS_WINDOWS_
Expand Down
20 changes: 20 additions & 0 deletions test/file.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,26 @@ b = mmap_bitarray((17,19), s)
close(s)
b=nothing; b0=nothing; gc(); gc(); # cause munmap finalizer to run & free resources

# mmap with an offset
A = rand(1:20, 500, 300)
fname = tempname()
s = open(fname, "w+")
write(s, size(A,1))
write(s, size(A,2))
write(s, A)
close(s)
s = open(fname)
m = read(s, Int)
n = read(s, Int)
A2 = mmap_array(Int, (m,n), s)
@test A == A2
seek(s, 0)
A3 = mmap_array(Int, (m,n), s, convert(FileOffset,2*sizeof(Int)))
@test A == A3
close(s)
A2=nothing; A3=nothing; gc(); gc(); # cause munmap finalizer to run & free resources
rm(fname)

#######################################################################
# This section tests temporary file and directory creation. #
#######################################################################
Expand Down