Comparison and object operations. - #2475
Conversation
gbrail
left a comment
gbrail
left a comment
There was a problem hiding this comment.
Both of these small things were found by AI but I verified them. Both affect performance but after all that's what we're trying to accomplish here. If you'd rather take care of them some other time (or not at all) that's OK too.
| { | ||
| number_compare: | ||
| { | ||
| Number rNum, lNum; |
There was a problem hiding this comment.
if DOUBLE_MARK is true for both lhs and rhs we are missing a big optimization opportunity to do the comparison without any boxing. This would require adding a "compare" function in ScriptRuntime that takes two doubles as arguments, but it would prevent us from undoing the optimizations that we got. OTOH I don't know how much the interpreter uses the ability to avoid boxing of doubles, since the compiler does that by generating different bytecode.
|
|
||
| @Override | ||
| public void interpret(Context cx, CallFrameV2 frame) { | ||
| frame.push(ScriptRuntime.typeofName(frame.scope, name)); |
There was a problem hiding this comment.
ScriptRuntime.typeofName calls Context.getContext(). Since you have the context already you can save a thread-local variable lookup if you use "cx".
b62132b to
0a66ebe
Compare
|
Thanks for spotting those @gbrail, while fixing them I also moved a couple of switch statements over to switch expressions, since I was touching those areas of the code anyway. |
0a66ebe to
86675fb
Compare
|
BTW @gbrail I got my robot friend to apply the review comments from this and the previous instruction PR to all of the other instruction commits. I've put everything on https://github.com/aardvark179/rhino/tree/aardvark179-interpreterv2 (the branch for #2426). I don't see the test failure on that PR locally, but I have found various ways to push similar scripts over the edge and cause a stack overflow in node transformation—I can cause the same issues with V1 and class compilation. I'm going to try some refactoring to reduce the problem in node transformation and then see if I can provoke the issue in code gen as well. |
|
Thanks, looking good! |
2 participants
Next set of instructions, comparison, equality, and types and instance of operations.