Closed Bug 843937 Opened 9 years ago Closed 9 years ago
Monkey: specialize on eval("name()")
As outlined in bug 842522, the eval's in ss-date-format-tofte are of the form eval("name()"). It would be nice to specialize on this and skip the construction and entering of the eval script, which will just call back into a regular function. The attached patch does this for IonMonkey. I get these times on a date-format-tofte modified to run 20x longer: d8: 167 trunk, pre-743394: 450 trunk, post-743394: 385 trunk, with patch: 310 So, this plus 743394 cut out about half of the difference between us and v8. I believe the remainder is primarily (a) a lack of inlined natives for Date functions like getHours(), and (b) a lack of ggc (tofte's main function creates a lot of garbage arrays and lambdas). Of course, date-format-tofte runs short enough that the benefits of Ion optimizations will not be fully realized. On trunk the main function in tofte is not compiled by Ion until practically the end of the benchmark, so there won't be any benefit to doing this. With the baseline compiler, we compile much sooner so the main function should (need to double check) run about 90% of the time as Ion code. This patch is pretty woefully special cased to the format of the eval calls in tofte. It would be nice to beef this up at some point and deal with other simple eval forms (eval("name"), eval("obj.prop"), ...), but doing so would require feedback from baseline about what sort of strings have flowed through the eval previously. Also, please pay attention to the use of resume points, safepoints and snapshots around the VM call made to do a dynamic name lookup. This call can't fail or invalidate, but still seems to require the associated machinery (is there a way to avoid this?) and has a bailout afterwards.
(In reply to Brian Hackett (:bhackett) from comment #0) > Also, please pay attention to the use of resume points, safepoints and > snapshots around the VM call made to do a dynamic name lookup. This call > can't fail or invalidate, but still seems to require the associated > machinery (is there a way to avoid this?) and has a bailout afterwards. You can use masm.callWithABI instead of callVM if the VM call cannot GC. It's a bit faster and doesn't require a safepoint, resumepoint etc. See for instance LModD codegen.
Updated to use callWithABI instead of callVM. This improves the time from 310 to 300.
Comment on attachment 717094 [details] [diff] [review] updated Review of attachment 717094 [details] [diff] [review]: ----------------------------------------------------------------- Nice. ::: js/src/ion/CodeGenerator.cpp @@ +1684,5 @@ > + masm.adjustStack(-int32_t(sizeof(Value))); > + masm.movePtr(StackPointer, temp2); > + > + masm.setupUnalignedABICall(4, temp1); > + masm.passABIArg(temp1); This should be temp3.
Attachment #717094 - Flags: review?(jdemooij) → review+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.