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

copy() for Tasks #4085

Closed
JeffBezanson opened this issue Aug 16, 2013 · 11 comments
Closed

copy() for Tasks #4085

JeffBezanson opened this issue Aug 16, 2013 · 11 comments
Labels
speculative Whether the change will be implemented is speculative

Comments

@JeffBezanson
Copy link
Member

This needs to be implemented in task.c. After copying all the fields of the jl_task_t, the following extra changes are needed:

  • deepcopy tls
  • set consumers to jl_nothing
  • set donenotify to jl_nothing
  • allocate with allocb a copy of stkbuf, which is bufsz bytes

I think there is a chance that will work.

@JunaidBKirkire
Copy link

@JeffBezanson Would like to take a crack at it.

@pao
Copy link
Member

pao commented Aug 29, 2013

@JunaidBKirkire go for it! Just make sure that your pull request references this issue by including the (unquoted) string #4085 in the description.

@StefanKarpinski
Copy link
Member

Do we really need to deepcopy task local storage or is a copy sufficient? It seems to me like a deepcopy is overkill.

@JeffBezanson
Copy link
Member Author

Shallow copy is probably the right thing.
On Nov 13, 2013 4:58 PM, "Stefan Karpinski" notifications@github.com
wrote:

Do we really need to deepcopy task local storage or is a copy sufficient?
It seems to me like a deepcopy is overkill.


Reply to this email directly or view it on GitHubhttps://github.com//issues/4085#issuecomment-28437875
.

@vtjnash
Copy link
Member

vtjnash commented Dec 21, 2014

also would need to check whether the task being copied is jl_current_task

the user asked for a deep-copy, so it seems like that would mean tls also. otoh, if the user just copy()'s the task, then a shallow copy seems like the right thing.

it's a bit hard to implement this without having an example use case to use as a test

@malmaud
Copy link
Contributor

malmaud commented Oct 11, 2015

I doubt anyone has tried exactly the use case of cloning tasks for probabilistic programming in Julia. @zenna has probably done the most wrt prob prog in julia; I know a fair amount about how coroutines are implemented in Julia but not enough to implement this feature myself. Let's move the general discussion of tasks for probprog to the forum and leave this issue specifically about the copy functionality.

@vtjnash
Copy link
Member

vtjnash commented Feb 14, 2016

copying here my concerns from #12625 (comment) that I would want to see addressed first. In particular though, I would be very against merging something to close this issue that in any way adds complexity or difficulty to the implementation of the very important gc-optimizations mentioned previously.

au contraire, without a use case, it is a lot of work to add and maintain a feature that will always be suboptimal relative to using a closure or explicit work queue. I suspect this feature may interfere with stack-allocating objects, gc-frame elision, and other allocation-scope optimizations since it destroys the assumption that a stack frame is non re-entrant. Also, this does not strike me as a good way to implement backtracking, except as an academic exercise or toy demo.

to add to that, I also feel that it is disconcerting that the copy / deep-copy behavior is essentially unpredictable.

@vtjnash vtjnash removed the help wanted Indicates that a maintainer wants help on an issue or pull request label Feb 14, 2016
@yebai
Copy link

yebai commented Feb 15, 2016

The code contained in PR #15078 has been used for a few months in a probabilistic programming library for Julia. The observed behavior of this implementation has been so far stable and predictable in our experiments. As mentioned in PR #15078, this implementation duplicates the target task's stack. So, in principle, any GC optimisations applicable to the target task can be repeated on the copied task's stack.

@yuyichao
Copy link
Contributor

Having the code working for a few month in a very limited usage pattern is relatively easy and basically doesn't really mean anything. There's a fundamental difference between having something that kind of works (i.e. works for a single usage pattern) and providing it in the standard library. Just to name a few obvious things that a task cannot do if you want to copy it.

  • It cannot cache a reference to itself. (i.e. it need to get the current task object again after anything that can cause a task switch)
  • It cannot cache a reference to a task local object (same with above)
  • It cannot hold a lock, (i.e. it cannot print, at least the caller has to make sure this is the case when a task is being copied)

This also kills !COPY_STACK since there's no way you can fix a pointer to a stack allocated object (yes, the pointer is on the stack but the stack is not type safe and you don't know what bit pattern needs to be fixed.).

@yebai
Copy link

yebai commented Feb 17, 2016

These are valid concerns indeed. However these issues have been met and solved in situations where fork is implemented: the most obvious example is UNIX processes. So, we can re-uses ideas that have been proposed for solving similar problems encountered when forking processes.

For example, the first two issues are solvable by avoiding caching self reference in task related code. The 3rd issue is more tricky, but it is easy to add checking in task-switching code to enforce atomic behaviours: tasks that possess a lock (or other critical recourses) must finish their operations and release the lock before passing control to other tasks.

The last issue related to !COPY_STACK is more difficult. But it is still solvable, for example, by creating a backup of the a task's stack and copy it back before switching back to the corresponding task. This idea is not perfect but has a similar spirit with the COPY_STACK method, which is the current default implementation for tasks in Julia.

@yuyichao
Copy link
Contributor

Yes, this has all the problem with fork and is much harder to solve even though the idea might be similar. The issue with fork is that it is likely broken whenever IPC is involved. It is a solvable problem for fork since most of the operations in a process are local and when you explicitly ask for IPC, you potentially know how to write the correct atfork hooks (even then, many people don't do that and it is very common for a forked process to be broken if it want to do something non-trivial). With tasks, the story is totally different. Since everything runs in the same memory space, everything you do is potentially an intertask communication.

For example, the first two issues are solvable by avoiding caching self reference in task related code.

I don't think this is practical. It's hard enough to predict when a task switch can happen and we shouldn't make is much harder to write code that is robust against task switch.

Getting this to work will involve adding code to deal with this unlikely case that will unnecessarily slow everything else down. Given what you can do by coping a running task should be much cleaner to be implemented as cloning a stateful generator (iterator), I don't think it makes sense to implement this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
speculative Whether the change will be implemented is speculative
Projects
None yet
Development

No branches or pull requests

9 participants