-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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.
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
RFC: Add windows support for shared memory and SharedArray. #8797
Conversation
This builds and passes tests on both win32 and win64 on my machines, very nice. I'm not sure I like the following though:
Would it hurt or make any difference to use the wide-char versions Maybe the way you have it now is the only practical way of doing this, or others think it's fine, or I'm being needlessly picky/conservative. Not sure. |
If by this you are referring to, for instance, None of these "changed" functions are part of the current public interface (they are implementation details of
Oversights. I will fix these.
The return code is not used in I could see it making sense, though, to remove the raising of the error and return a
I guess I'm still used to the "old-school" means of defaulting in Julia (sooo 2012!). The last significant coding I did in Julia predates the addition of default arguments. I'll update accordingly.
I'm not sure I understand exactly what you mean here. Maybe you can elaborate. Both POSIX and windows support distinct concepts for "get shared memory" and "memory map it". Aiming for the largest common denominator in the low-level interface, I wanted to make sure it reflected this commonality. I looked to the Netscape Portable Runtime (NSPR) Named Shared Memory API (and its implementation) for inspiration. It was my intention that its semantics are roughly duplicated in Julia. It would have been nice if Windows supported a stream-like abstraction for shared memory, as POSIX does, in which case we wouldn't need a new I'm certainly open to a more elegant solution.
Yes, there is very little shared code. However, that little bit captures the most common use case: get a chunk of shared memory then mmap all of it. The lower-level interface supports doing it in two steps, in case you want to mmap a smaller chunk of it or perform multiple mmaps on the same chunk. The vast majority of the time, though, you will want to do it in one fell swoop (as Do you find the overloading itself confusing? If so, I'm certainly open to a renaming.
POSIX |
Having thought about it a bit more, I think there might be a way to eliminate the explicit "two-step" nature of the low-level interface without sacrificing the flexibility I was after. I think it would also mean that there wouldn't need to be a separate |
The fact that Julia doesn't have truly private functions means you never know who might be hooking directly into some unexported functionality for whatever custom reason they had. It seems Julia caters to the type of user who tinkers with internals more often than you'd think :) PkgEvaluator will find these things for registered packages, and API breaks are fine especially a) for unexported functions and b) during the dev period. But if you're changing names and interface it does seem like there should be a good reason. These are still pretty close to the posix interfaces, except that the names are different now and the mode is split into 2 booleans.
Oh. You've been around a lot longer than I have then. :)
Yeah, I noticed that too, having touched that piece of code a little while ago. Maybe a comment noting that some of the implementation is common, if the code you're copying has any bugs then we'll need to remember to check both places.
Mostly a concern about limited namespace and generality. A type called
Yeah, I did. It feels as though it might be more clear to me if you didn't have separate "inner" versions of
If the name is a
Sounds like we might be going the same direction then. Good stuff. Apparently when I said "kick the tires" what I actually meant was "take apart the engine." Do you or anyone else happen to have any interesting benchmark code using SharedArrays that we could look at performance on? |
That
I do not. Getting cross-platform support for
I think we are going to have a significantly better implementation for your input. |
I think if you convert the string to utf16 before sending to to ccall, then ccall should do the marshalling. There are some examples of using wide-char winapi functions in env.jl and file.jl. Testing is a good way to find out. |
|
||
A = mmap_array(T, dims, s, zero(FileOffset), grow=false) | ||
catch e | ||
print_shmem_limits(prod(dims)*sizeof(T)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should retain the informational printing of shared memory limits on an error. Will help folks debug errors due to system limits faster.
Sorry for the delay in responding. Windows support is a nice addition, but the Windows API is not something I am familiar with. Other than my comments on the error handling bit above, LGTM. |
@amitmurthy Thanks for taking a look. I'm currently working on a different branch which attempts to preserve as much of the existing code as possible, including the bits you mention. Do you have any non-trivial examples of the use of |
No, I don't have any. Maybe @timholy can help? |
I do, but they're integrated into a massive application that uses Images, CUDArt, and a lot of custom code, and wouldn't be at all trivial to separate out into a benchmark. Better to write one from scratch. |
I'll probably write up a toy Poisson solver example at some point then. |
Superceded by #9044. |
base/shmem.jl
).SharedArray
to use this new interface.test/parallel.jl
runs successfully on 32-bit and 64-bit windows and 64-bit linux.