JM does not compile try..finally

RESOLVED WONTFIX

Status

()

defect
RESOLVED WONTFIX
8 years ago
6 years ago

People

(Reporter: jandem, Unassigned)

Tracking

(Blocks 1 bug, {perf})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [games:p-][js:p?])

(Reporter)

Description

8 years ago
JM does not compile the following function:
--
function f() {
    try {} finally {}
}
f();
--
It aborts with:
[jaeger] Abort    couldn't analyze bytecode; probably switchX or OOM

Looks like jsanalyze.cpp does not handle JSOP_FINALLY.
(Reporter)

Comment 1

8 years ago
I don't see this in SS/V8/Kraken, but I can imagine other benchmarks/website using it.
Blocks: WebJSPerf
No longer blocks: Jaeger
(Reporter)

Updated

8 years ago
Blocks: 566369
No longer blocks: WebJSPerf
Duplicate of this bug: 782909
Do we need a corresponding bug for Ionmonkey?
It's probably not worth filing until something comes up. IonMonkey doesn't even support try/catch since it's sort of non-trivial, due to a combination of the compiler design and JS's variable scoping. (It can be implemented, but it hasn't come up yet so the work hasn't been scheduled.)
Following up from Kevin's email to the games list. I think the first questions are for Kevin:

- It sounds like dispose methods are very popular in C# (which makes sense--it's a good feature), so you need a way to support them well. Correct?
- Is try/finally the only way to implement them, or is there something else you could do?
Whiteboard: [games:p?][js:p?]

Comment 6

7 years ago
There are some things you can do to try to emulate the disposable pattern, but you still need a try block somewhere eventually.

If you statically prove that the code you're calling can't throw, you could simply convert the try/finally into normal invocations of the finally block. But then you need to ensure that you update each exit point (return, goto, etc) to call it too.

If you prove that the dispose block doesn't actually do anything, you can remove it. I do that. But that falls over for dynamic code.

You can wrap the contents of every try block in a function of their own, and utilize closures to allow variable values to change across the function border, etc. But even if that lets the inside of the try block get jitted, the outside can't be, so now you need like three layers of functions and the closures are going to create garbage. I've seen this method recommended a lot, actually.

Comment 7

7 years ago
Oh, sorry, and yes, Dispose is unfortunately super common in C#. The 'foreach' construct implicitly has a Dispose at the end of each loop (because iterators are Disposable), and that actually needs to be a try/finally instead of just a call at the end because it's pretty common for loop bodies to throw.

One could possibly argue that the try/finally pattern in JS shouldn't be about exception handling, since in JS very few resources actually need to be finalized to avoid leaks. But there are probably still cases where this matters - object pooling strategies designed to avoid garbage, virtual handles to resources living on a server somewhere, etc.

Updated

7 years ago
Blocks: gecko-games
IIUC, the main difficulty in compiling try blocks is that if you build the CFG in the obvious way, you get an edge from every instruction in the try block to the finally block. I'm not sure exactly what's bad about that but I think it means potentially many side exits (which use memory), and makes optimizations run more slowly, and especially can complicate register allocation.

If you build the CFG in the slightly less obvious way, you get an edge from every fallible instruction to the finally block. I don't know if that's few enough to make it work or not.

But, it seems to me that if the original code was from the dispose pattern, then you're not necessarily expecting to throw any exceptions, but rather just want to run the finally block if you do an early return or break. So, JIT people, what about just cloning the finally block along every "normal" CFG edge that goes outside the try block and taking a side exit if an exception is thrown in the try block? Would that be efficient and optimizable?
In IonMonkey it would be fairly simple to compile try and finally blocks, but ignore catch regions. An error would cause a bailout, and the finally block would have a single incoming edge from the bottom of the try.

I don't think there is any way that "catch" is going to be fast unless it comes from a "throw" keyword. If that's the case, we can treat throws from C++ as exceptional, causing a bailout in IM. Meanwhile explicit "throw" instructions would act as direct edges to the catch block - no problems there.

I think an initial step though is, this bug: getting JM to support finally blocks, since we really want JM to run before Ion.
Our bytecode has some super weird GOSUB/RETSUB thing to make finally work. Is that needed? In a few tests I made, we generate stuff like:

   try
   ...
   gosub X
   goto Y
   <catch>
   gosub X
   goto Y
X: finally
   retsub
Y: stop

Could this just be:

   try
   ...
   goto X
   <catch>
   goto X
X: finally
   stop

Or am I missing something?
(In reply to David Anderson [:dvander] from comment #10)
> Our bytecode has some super weird GOSUB/RETSUB thing to make finally work.
> Is that needed?

That sounds like what JVM bytecode does for try-finally.  In general, you can use code duplication to eliminate finally blocks.  That's basically what you are doing in this case.

I've read that a study of a large body of JVM bytecode that the increase in code size was trivial, despite the fact that this can cause exponential (or whatever) code blowup in the worst case.  (This was in the context, I believe, of arguing that maybe they should have done that to simplify bytecode verification.)
So you could do something like put an upper bound on the nesting depth of try-finally blocks you support (maybe 2 or 3) and just interpret beyond that, or something.
Whiteboard: [games:p?][js:p?] → [games:p1][js:p?]
Whiteboard: [games:p1][js:p?] → [games:p-][js:p?]
JM was removed.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.