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.

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

Converting WrenHandle to a foreign object's data #592

Closed
avivbeeri opened this issue Aug 2, 2018 · 17 comments
Closed

Converting WrenHandle to a foreign object's data #592

avivbeeri opened this issue Aug 2, 2018 · 17 comments

Comments

@avivbeeri
Copy link
Contributor

If I have a foreign method which is being passed a foreign object, then:

a) Does it make sense to create and hold onto a WrenHandle* for it, especially if I want to return the object later as part of a foreign getter

and b) Can I extract or modify the foreign data from the WrenHandle*?

@mhermier
Copy link
Contributor

mhermier commented Aug 3, 2018

a) yes you have to garantee life time
b) depends on when you want to modify.
The normal way is to push back to a stack to get the raw pointer.
An alternative way is to store the WrenHandle in the object. You can always do:

struct Foo {
   WrenHandle* self;
   ... payload ...
}

And make the holder object to handle Foo*.

@avivbeeri
Copy link
Contributor Author

I thought about using the stack/slot array, but this is required in some threaded code so it isn't safe to do that.

Embedding the WrenHandle inside it's own foreign object... that's interesting indeed.

@mhermier
Copy link
Contributor

mhermier commented Aug 3, 2018

Well since you said you are in threaded code, then you will surely have to make some synchronisation around destruction, and don't try to call wren code directly from worker thread. All the trivial threaded stuff...

@avivbeeri
Copy link
Contributor Author

avivbeeri commented Aug 3, 2018

It's my first time writing serious threaded code so that's a whole learning experience, but yes, there is a separation of concerns, and I'm definitely not calling Wren from the threads. I wanted asynchronous IO for my game engine, so the threads are handling the IO operations, and the main thread runs the wren vm. The difficulty is getting the results back to wren safely.

At the moment, I'm basically creating a Result object, passing that to a foreign method which spawns the thread. So you need to make sure the Result actually sticks around until the thread completes, and that it can modify the data inside.

I think your Foo example above would be suitable for that.
(Hopefully all that made sense...)

@mhermier
Copy link
Contributor

mhermier commented Aug 3, 2018 via email

@avivbeeri
Copy link
Contributor Author

avivbeeri commented Aug 3, 2018 via email

@ruby0x1
Copy link
Member

ruby0x1 commented Aug 3, 2018

The "main" thread is a concept. If you enter main and make and join new thread, you can call that main (even though it is the 'second' thread in the application). Then you do all the normal stuff you would do, but you have the 'main' thread still. Or word it differently, "game" thread and "io" thread, the order in which they are made doesn't matter.

I don't see the need to make it more complicated than it is, though. It's fine to keep it that way with a worker thread for IO. This is very common. Typically for async IO you just need a simple array of events/messages with a lock on adding to it. It doesn't have to be fancy (so no need for lock free) since async IO is not a 10000 per frame type of operation that needs low contention. Find or make a simple thread safe queue or list, a mutex on adding to the list is fine. Reading shared data from multiple threads is fine, only writing causes the concern.

Then send the payload into the queue, and the main thread will be checking it each tick or periodically (polled) for new events. When an event comes in, now it's handled on the right thread. You can grab the foreign out of the payload (assuming it's a call handle) and use it safely since you're back on main.

@avivbeeri
Copy link
Contributor Author

Right. So assuming I've understood the whole conversation correctly, I can dispatch the IO task to my pool of threads. The task payload can contain the foreign object pointer with safety (having made a WrenHandle earlier), and then on a thread, modify the object data, post the results to the main thread and release the handle.

Assuming that makes sense, thank you both for your help.

@ruby0x1
Copy link
Member

ruby0x1 commented Aug 3, 2018

Not sure about modifying the object part, if you mean modifying the data owned by wren - that wouldn't be the right thing to do from another thread. A handle is not a lock on the data right.

@mhermier
Copy link
Contributor

mhermier commented Aug 3, 2018

Well there is a little thing to nuance, on unix systems (at least) the main/root thread is the only responsible to handle some process critical signals. This is why I think it is better to offload the wren thread to its own thread, so it doesn't need to deal with that directly.
Thought it should not change things much. I think I prefer not to have the interpreter thread to be interrupted by the system, and have it to handle a hard shutdown.

@avivbeeri
Copy link
Contributor Author

If I can't use the foreign data pointer directly from a thread, I'll just have to defer the result writing until the task completed and returns to the main thread. Not the end of the world, just an inconvenience.

@ruby0x1
Copy link
Member

ruby0x1 commented Aug 3, 2018

it just depends who owns the data and how it's allocated, the payload going back and forth can be for example two pointers, one for the block of data that the IO operation interacts with on both sides (making the payload itself the owner) and the handle.

If you allocate that block via wren (say a string as bytes) then you don't want to touch it via threads. If you allocate in the worker thread and the receiving thread just reads / copies it to wren that's fine. If the data is allocated inside the payload itself same thing, writes are not overlapping with reads (because you'd never read a payload from main thread while it's in the worker thread, it tells you when it's done and then you read it).

I think if you use setSlotString/Bytes currently wren takes a copy anyway and owns it. You could also probably mutex on the data of the bytes from the handle if careful but if you're new to threading it's best to keep it simple, make it work, and learn that way.

@mhermier
Copy link
Contributor

mhermier commented Aug 3, 2018 via email

@avivbeeri
Copy link
Contributor Author

Thanks all for your advice. I've managed to get to a point where I'm only holding onto a WrenHandle* and then using the Slot array on the main thread to retrieve a foreign data pointer from the Handle, which made me notice a slightly interesting thing:

There doesn't seem to be a safe way to release a WrenHandle that a foreign class object is holding, without doing it pre-emptively. My hope was to release it during my finalizer method (despite the warning that you have limited access to the VM).

On the otherhand, not releasing the Handle doesn't seem to cause any errors or other issues, so I suspect it's just a wasteful memory leak until the VM shuts down.

I guess the fact that I don't get an error or warning is a bug, given the explanation in the documentation.

@ruby0x1
Copy link
Member

ruby0x1 commented Aug 5, 2018

There is an error if you're in debug mode

@avivbeeri
Copy link
Contributor Author

That would answer it. Need to figure out a neater solution to that then. :/

@mhermier
Copy link
Contributor

mhermier commented Aug 6, 2018 via email

@ruby0x1 ruby0x1 closed this as completed Jun 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants