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

No finalizer #189

Merged
merged 3 commits into from
Dec 10, 2018
Merged

No finalizer #189

merged 3 commits into from
Dec 10, 2018

Conversation

joelfrederico
Copy link
Contributor

@stevengj
Copy link
Contributor

stevengj commented Dec 8, 2018

Needs tests.

@joelfrederico
Copy link
Contributor Author

Good call, I'll add some tests.

@joelfrederico
Copy link
Contributor Author

I'm coming to the conclusion that this library really needs to be reworked to not use finalizers for resource management. Or rather, to use them properly. We really do want to leak memory when sending data, and we want to do a ccall that gives libzmq the c-function that will delete that memory. We need to trust libzmq to do that properly. It's not really proper to need libuv in order to write libzmq bindings... right?

But at least I have written the tests.

@joelfrederico
Copy link
Contributor Author

I think I need to take back the bit about not using libuv. I read the docs a bit more and I see that GC and libuv are intertwined more than I appreciated.

@stevengj
Copy link
Contributor

You have two options:

  1. you can let libzmq make a copy of the data, which it can then dispose of as it wants. This is good for small messages, and is what happens with send(socket, x).

  2. for large messages, you can let libzmq use your buffers without making a copy, but then you need to protect the buffer from garbage-collection, and you need libzmq to release control in a thread-safe way. This is what happens when you send(socket, Message(x)). It currently has to involve libuv in Julia because libuv is what controls the event loop. As Julia gets more thread-safe constructs, it may be possible in the future to release the memory without involving the event loop. (Maybe it is possible now?)

@stevengj stevengj merged commit c071980 into JuliaInterop:master Dec 10, 2018
@joelfrederico
Copy link
Contributor Author

From what I read it isn't possible to do that (yet, anyways).

I wish I could release the buffer from the garbage-collection system entirely and manage it through C calls, basically get a pointer and permission to do a C-style free(void*). But I don't see that happening unless Julia's memory model specifically does C-style malloc and free calls in its implementation. I have no idea what Julia's memory implementation looks like, but at least they haven't exposed that in any documented API I know of. At least free(void*) is thread-safe so it's technically possible.

@yuyichao
Copy link
Contributor

I read the docs a bit more and I see that GC and libuv are intertwined more than I appreciated.

Huh??? What are you talking about? The GC is completely unrelated to libuv. It knows how to scan tasks as well as a few other special non-native julia objects and that's as much as how much it interact with ANY other systems. It's basically the one system that is the least "intertwined" with anything else, since basically everything else uses the GC but not the other way around.

@joelfrederico
Copy link
Contributor Author

@yuyichao See:

It currently has to involve libuv in Julia because libuv is what controls the event loop.

Also see https://docs.julialang.org/en/v1/manual/calling-c-and-fortran-code/#Thread-safety-1, specifically:

The callback you pass to C should only execute a ccall to :uv_async_send, passing cond.handle as the argument, taking care to avoid any allocations or other interactions with the Julia runtime.

Note that events may be coalesced, so multiple calls to uv_async_send may result in a single wakeup notification to the condition.

The GC isn't completely unrelated to libuv. Those are libuv calls to the event loop that should direct GC to delete data it holds (when it deigns to).

@yuyichao
Copy link
Contributor

yuyichao commented Dec 10, 2018

Where does anything you quote mention even a single time about the GC?

It currently has to involve libuv in Julia because libuv is what controls the event loop.

Yes? But this isn't GC related. libuv controls the event loop, yes. Neither the event loop nor libuv has anything to do with the GC. The GC only works when you are allocating, it doesn't care if there's a event loop or not.

Those are libuv calls to the event loop that should direct GC to delete data it holds

No.

@yuyichao
Copy link
Contributor

Those are libuv calls to the event loop that should direct GC to delete data it holds

Or if you mean, libuv uses the GC to allocate data, which will obviously also be free'd by the GC then yes, but that's a single directional interaction, it never direct the GC to do anything special, it's just a user of the GC, much like anything else, so I don't see what's intertwined.

@joelfrederico
Copy link
Contributor Author

Hey, calmate!

Libzmq is based around zero-copy. It would be ideal to take data allocated in Julia and pass it into ZMQ. ZMQ holds the data, but it doesn't know how to delete it. You do that by passing it a C function. Normally, that C function would straight delete the data.

There are two issues with that:

  • GC doesn't expose an API for deleting data.
  • Furthermore, even if it did, GC isn't thread-safe (as I read it).

So you couldn't just call into GC directly, you have to schedule a GC call. That requires interacting with libuv so that GC gets called in a thread-safe manner within Julia. Really, the recommended manner of "telling" GC to delete something isn't actually to tell GC to delete something, it's to call a function that gets rid of some references to the data. Then GC is free to delete the data whenever it wants to.

@yuyichao
Copy link
Contributor

yuyichao commented Dec 10, 2018

GC doesn't expose an API for deleting data

That is not the job of the GC. Such concept doesn't even exist in the GC. You stop referencing that object and the GC will delete it when it feels like it. Put it in a dict/set if you want to keep it for longer and that's what everyone do.

Furthermore, even if it did, GC isn't thread-safe (as I read it).

It IS thread safe. All what you read is that the julia runtime, NOT the GC, doesn't support external thread. Again, it has nothing to do with the GC. The part you link really doesn't mention anything about the GC so I still don't know where you get that idea from......................

you have to schedule a GC call

And there really isn't anything as a GC call, you are just running normal julia code to deal with normal julia objects...........

That requires interacting with libuv so that GC gets called in a thread-safe manner within Julia

No. "That requires interacting with libuv so that julia runtime gets called on a managed thread". The GC supports all the thread related stuff any part of julia does.

recommended manner of "telling" GC to delete something

That's not the recommended manner, that's the only possible way and there will never be a different way.

Again, if you can't wrap your mind around how a GC actually manages memory (I didn't at first) that's perfectly fine point to complain and learn about.
If you are not happy with the current support of unmanaged thread that's a very good point too, there's even an issue for it JuliaLang/julia#17573.

If you combine the two completely unrelated issue together just because you happen to hit both at the same time and call "GC and libuv are intertwined more than I appreciated" that's just wrong. I agree with almost all the direct observations you have and it's just this wrong conclusion that I'm objecting. For me, having the GC and libuv intertwined to any degree (in a user visible way at least, using a different main loop inside the GC if the needs arise would be perfectly fine) is a completely stupid, non-sense and unacceptable design, given the current overall design of the language so I want to make is absolutely clear that the interaction between the two is only single directional. (And as I just realized, not so clear from your initial statement, that the two issues you have are just unrelated to each other)

@joelfrederico
Copy link
Contributor Author

joelfrederico commented Dec 10, 2018

@yuyichao Are you trying to piss me off? Because you're succeeding.

For starters, caps and bold is bad internet etiquette equivalent to yelling. I'm sure you know that, knock it off.

Don't be rude by saying I can't "wrap my mind" around something. You know that's loaded language. You know that would piss you off if somebody said it to you. So don't say it to somebody else.

Next, I don't assume something is thread-safe unless somebody explicitly tells me it is. Nowhere do I read in the docs that the GC is thread-safe. You may have some more intimate knowledge about Julia's GC, but if it's not documented to be thread-safe then I will assume that isn't part of its contract. Even if its current implementation is thread-safe, I don't want to get burned later if it becomes unsafe.

Finally, don't just say, "No," and be condescending. Ask what I meant when I said that libuv and GC are intertwined. I think they're intertwined because I can't do anything GC-related unless I'm doing it in a thread-safe manner because AFAICT GC isn't promised to be thread-safe. That's literally all I meant by "more intertwined." I'd previously hoped I could have an API directly into the GC I could use to say, "this variable may have zero references later, but hang on to it until I tell you otherwise" (like @preserve effectively does but non-locally). Then in a separate thread-safe C function call later have an API that says, "You know how I wanted you to hang on to it? I'm done now, you can go back to normal operation."

I'm moving on. This all is moot. I know how to prevent GC from deleting my data before I want it to and I know how to signal to the GC that the data is ready to be deleted, and I know how to do that in a thread-safe manner using libuv because I can't trust the GC to be thread-safe. You can rant if you like but I'm done.

@joelfrederico joelfrederico deleted the no-finalizer branch December 11, 2018 00:21
@yuyichao
Copy link
Contributor

yuyichao commented Dec 11, 2018

For starters, caps and bold is bad internet etiquette equivalent to yelling. I'm sure you know that, knock it off.

No I don't. Those are emphasizing the important points, i.e. the points I'm directly objecting, and make them stand out from the long reasoning behind that point.
(edit: on a closer check of what I see caps and bold anywhere, note that I was only using single word caps, I do not believe they are equivalent to yelling. So I do not believe my use of them are inadequate.)

I can't "wrap my mind" around something. You know that's loaded language. You know that would piss you off if somebody said it to you. So don't say it to somebody else.

I actually don't mind someone saying that to me because I'm very sure there are a lot of things I can't wrap my head around. Well sorry about that and noted. I'm actually commenting on you asking for a free(void*) in a GC, which is just not how a GC works.... It's not a limitation of the GC implementation.

Next, I don't assume something is thread-safe unless somebody explicitly tells me it is. Nowhere do I read in the docs that the GC is thread-safe. You may have some more intimate knowledge about Julia's GC, but if it's not documented to be thread-safe then I will assume that isn't part of its contract. Even if its current implementation is thread-safe, I don't want to get burned later if it becomes unsafe.

That's really not how it works because if GC isn't threadsafe then literally nothing is. It's not even a user visible part so there's no need to document about an implementation detail in user document. It's extensively documented for developer (comments in gc.c). If there was a function to free any memory for the user, then yes, that should be documented. Right now there's just nothing to say about it.

Ask what I meant when I said that libuv and GC are intertwined

I usually do when I feel like there's even a little bit of ambiguity but by all mean libuv and GC are not intertwined. Your use case needs both and that's it. If you said instead sth like "interacting with julia/GC from an external thread involves libuv more than I appreciate" that's perfectly fine (edit: and I'll even agree). As far as I understand, and please correctly me if this is a wrong understanding of the sensence, "libuv and GC are intertwined" is describing the libuv and GC, rather than a specific usecase that needs both. And as I hinted above, the property described here makes so little sense here that it feels very insulting. (I thought about mentioning this more explicitly in the previous comment but removed it. I didn't know that the highlight pisses you off though.............................)

I'd previously hoped I could have an API directly into the GC I could use to say, "this variable may have zero references later, but hang on to it until I tell you otherwise" (like @preserve effectively does but non-locally). Then in a separate thread-safe C function call later have an API that says, "You know how I wanted you to hang on to it? I'm done now, you can go back to normal operation."

As I said above, there's no such API now because nothing is needed. It's actually much more efficient to do one dict per user since the lookup and scaning will be faster. The only, really only, reason this is now impossible to do from an external thread without libuv is because julia itself does not support external thread, again, not sure how to emphasize once again without bold, not the GC. FWIW, clearing a reference isn't even a GC operation......

I know how to do that in a thread-safe manner using libuv because I can't trust the GC to be thread-safe.

I'm not sure if you are using an alternative meaning of thread-safe here so please ignore if by thread safe you don't mean being able to use it from multiple thread without undefined behavior. If you do mean the thread-safety as what I thought you mean, then again, you aren't talking about thread-safety at all, you are talking about external thread support. A friendly note that you are actually relying on the GC to be thread safety all along.

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

Successfully merging this pull request may close these issues.

3 participants