avoid core.eval for error handling
#373
Conversation
| case ex if NonFatal(ex) => | ||
| Failure(ex) | ||
| } |
There was a problem hiding this comment.
There is no longer a need to match on NonFatal here - that's already checked in the loop.
| try | ||
| catchingLoop(v) | ||
| catch | ||
| case ex: Throwable if (NonFatal(ex) && pf.isDefinedAt(ex)) => | ||
| pf(ex) |
There was a problem hiding this comment.
This outer try/catch appears unnecessary if you make v a function, but from the description that seems intentional.
There was a problem hiding this comment.
It's placed to catch exceptions in the evaluation of the v thunk since it's a by-name parameter. Making it a no-arg function is equivalent to a by-name param in the generated bytecode and we'd still need to evaluate the function to decide if it's necessary to suspend or not (catchingLoop). Although these two catches seem close in the method, they're placed in very different execution paths, which is helpful to specialize the code for JIT compilation. This seems the key characteristic that enables the better performance in comparison to eval, which was making the execution become much more dynamic.
This is the main cause of the regression in #347. The
evalmethod introduces much more overhead than the previous dedicated loop. The issue is that function applications don't get inlined and producing a thunk to provide toresumeintroduces more indirection, preventing JIT optimizations.I'll post benchmark results tomorrow.