Stack limits and AST visitors

[Adam Klein]

In trying to use AstExpressionVisitor in the parser, I've found the need to use an AST visitor where I don't have an Isolate handy. The DEFINE_AST_VISITOR_SUBCLASS_MEMBERS macro uses the Isolate for exactly one thing (in the method CheckStackOverflow):

...

StackLimitCheck check(isolate_);

if (!check.HasOverflowed()) return false;

...

My question is this: is it important that the StackLimitCheck pull the stack limit out of the isolate every time CheckStackOverflow() is called? Or could we store the stack limit when AST visitors are constructed? The latter is what we already do for parsing, in ParseInfo. And from poking around the code (and in Blink) it seems like we generally only set the stack limit at thread startup. But I'm wondering if there are cases I'm not thinking about (maybe cases where a single AST visitor is used on multiple threads with the same Isolate?).

- Adam

[Adam Klein]

Any thoughts here? Caitlin is currently running into exactly the same need in her do expression patch, so it'd be good to understand if we can do something general here instead of having most AST visitors be Isolate-bound while the few used from the parser need special duplicated code for stack check handling.

[Jochen Eisinger]

We use the stack limit to request interrupts from different threads, so checking it on a regular basis is important

--
--
v8-dev mailing list
v8-...@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+un...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Adam Klein]

Which sorts of threads are expected to be interruptable? Clearly the parser isn't (currently) interruptable in this way, since it takes in a stack limit at construction time and never re-sets it.

I guess the question is: is interruptability something that _all_ AstVisitors should have, with a few exceptions? Or is it something that a few AstVisitors care about, but most don't need to. Answering this question will help decide what the right way of refactoring the base class stuff is.

[Jochen Eisinger]

The 'main thread' for a isolate is supposed to be interruptable, e.g., via Isolate::RequestInterrupt or Isolate::TerminateExecution.

I can't answer your question about how the parser / ast visitors is supposed to behave here :-/

[Jakob Kummerow]

Maybe I'm missing something, but AFAIK we have two very different kinds of stack checks:

(1) generated code contains stack checks at function entry and loop back edge. These are for interruptibility of long-running code sequences.

(2) AST visitors do stack checks at recursive calls to check for bailout conditions. When one of those checks fails, they cancel whatever they were doing, and can never resume. So these checks are not for being interruptible.

I think that means for Adam's original question: it should be fine to just store a stack limit to implement an isolate-independent AST visitor. (Many existing AST visitors will need an isolate for other purposes, however.)