Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Browse files
Browse the repository at this point in the history
[Truffle] Need to transfer because expensive construction of exceptio…
…ns in Float#to_i.
- Loading branch information
Showing
1 changed file
with
2 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6bd4b27
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using transferToInterpreter here means that the cases where Infinity and NaN occurs will always be run in the interpreter (which might be the expected behavior). Alternatively, one can also use a branch profile to remove the branches from the fast path and optionally adding them back if they actually occur for the application.
6bd4b27
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that we can't compile the Infinity and NaN cases at the moment due to some string operations and other things involved in constructing the exceptions. Ideally those things should get fixed or get put behind
TruffleBoundary
. In the mean time we should annotate why we are transferring in each case. Perhaps we should define some aliases, for exampletransferBecauseCurrentlyUnableToCompile
.6bd4b27
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. Can we make the constructions of those strings for exceptions lazy? Graal would be easily able to escape analyze a capturing closure. In this case it might be not so important, but there are always cases where programmers rely on exceptions for normal control flow ;).
6bd4b27
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes you're right - we do want all these cases to be on the fast path. One problem at the moment that we have is that in order to construct an exception we need to construct a backtrace object, and in order to do that we need to walk the frame stack to find all the call nodes and their source sections. I believe that requires a full deoptimization anyway. That's why I hadn't tackled this problem yet. Using the backtrace object is pretty common and isn't a corner case we can't ignore. For example the RubySpec runner we are using makes extensive use of it.
I believe there is some research on lazy deoptimization. So where we'd normally deoptimize to get a call node for a frame we have some kind of lazy object that gets forced either when we ask for it - or when the frame gets unwound so it wouldn't exist any more if you did force it.
6bd4b27
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can avoid the construction of the backtrace if Graal sees it is unused. Anyway, let's revisit fast exceptions at a later point in time. In the meantime putting the work of constructing the exception behind a
TruffleBoundary
seems the right thing to do.6bd4b27
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok - we'll be doing a big pass through the code after we've merged a few more branches after the 0.6 release, and we'll check on all the exceptions behind transfers then.