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

WIP: share the stack with JuliaInterpreter and add support for breakpoints #41

Closed
wants to merge 3 commits into from

Conversation

KristofferC
Copy link
Member

No description provided.

@timholy
Copy link
Member

timholy commented Mar 6, 2019

The alternative, of having JuliaInterpreter use stack everywhere, is also something I can consider. The hard part is that our way of handling Compiled() would presumably have to change.

@KristofferC
Copy link
Member Author

In some situations, not having the top-level frame be "special" (and just have it be the top frame in the stack) makes things a bit easier.

However, I would think a linked list of frames for the stack might be an even easier structure to handle, since the random access of a Vector is seldom useful. Might be too intrusive to change at this point though.

state.stack[end].last_exception[] = exc
return true
end
original_stack = copy(state.stack)
Copy link
Member Author

Choose a reason for hiding this comment

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

For example here, just being able to store original_frame = stack.frame and then restore stack.frame = original_frame would be nice, instead of having to both store the frame and the stack to restore the original state.

@timholy
Copy link
Member

timholy commented Mar 6, 2019

However, I would think a linked list of frames for the stack might be an even easier structure to handle, since the random access of a Vector is seldom useful. Might be too intrusive to change at this point though.

It's worth considering; you have a good point with regards to the cleanliness of the API.

If I'm thinking about this clearly, the caller would have to hold the "root," meaning the top-level caller; otherwise if an error gets thrown deeper in the stack, there is no way to propagate the state back to the caller. Would you have to traverse the whole list to do anything? Or would each recursive call pass only the immediate parent? I guess you must be intending a doubly-linked list? Otherwise you have to choose between performance (avoiding the need for routine whole-stack traversal) and the ability to print the whole stack for debugging purposes (e.g., those two commented lines at the top of _step_expr!).

A related question: if we go that route, how do you signal intent for the behavior of recursive calls? Currently we can pass stack or Compiled() as an argument. Now that I think about it, it is a little strange: stack should be holding the parents of frame, but in fact it affects the allocation of the child-frames.

I'm willing to help implement this, but let's discuss a little more first.

@KristofferC
Copy link
Member Author

I guess you must be intending a doubly-linked list?

Yes, I was thinking double linked.

I'm willing to help implement this, but let's discuss a little more first.

Yes, getting this right is important since it will shape the API and the convenience in using the interpreter significantly.

An argument against the linked list is that it gives more power to the frame that it doesn't really need. If you see a function f(::JuliaStackFrame) you know it won't change the stack. However, the fact that almost all functions have a signature like f!(stack, frame) makes this argument slightly weaker.

@timholy
Copy link
Member

timholy commented Mar 6, 2019

An argument against the linked list is that it gives more power to the frame that it doesn't really need. If you see a function f(::JuliaStackFrame) you know it won't change the stack. However, the fact that almost all functions have a signature like f!(stack, frame) makes this argument slightly weaker.

If you wanted that clarity, a design like this:

struct Node{D}
    head::Union{Node{D},Nothing}
    tail::Union{Node{D},Nothing}
    data::D
end

would allow you to pass node.data instead of the entire node.

In addition to frame and stack, we have a couple of auxiliary variables to consider:

  • pc: pass as an argument or stash in the frame data more often? I suspect @Keno created new frames rather than updating the pc because he wanted the ability to re-run the same frame? (Essentially making frames immutable.)
  • istoplevel
  • something to indicate the desired form of recursion (interpreted or compiled). This might be more than a Bool or simple flag; the exec! argument of evaluate_call! is essentially a mechanism for detailed control of recursion. We might consider whether we can pass a function argument, recurse, and have it handle all aspects of recursion.

@KristofferC KristofferC deleted the kc/use_stackzzzzzzzzzzzzzzzzzzz branch April 4, 2019 15:23
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.

2 participants