-
Notifications
You must be signed in to change notification settings - Fork 683
Optimize arithmetic in VM. #1173
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optimize arithmetic in VM. #1173
Conversation
@@ -1651,6 +1657,36 @@ vm_loop (vm_frame_ctx_t *frame_ctx_p) /**< frame context */ | |||
} | |||
case VM_OC_ADD: | |||
{ | |||
if (ecma_are_values_integer_numbers (left_value, right_value)) |
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.
For the newly added code to the VM_OC_ADD and VM_OC_SUB cases there seems to be a lot of code duplication (minus the actual add/sub operations and invocations of the respective helper functions). This probably could be factored out into a static inline helper function without a negative performance impact. Not sure it's worth the hassle though, I guess it's up to you whether you want to go that route :)
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.
Could work, but I need to make a separate patch to try that out and see what is happening.
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.
It seems the helper function requires several parameters, so it does not look really nice. Macros make hard to debug things.
The VM is the most critical element of the engine performance, so we use techniques here which is not used in the other parts of Jerry. E.g. gotos, and code duplications. I know this increases the maintenance burden (and binary size), but the gain here outweights the costs. For binary size optimizations, we usually focus on rarely used but large parts of the code, e.g. json support.
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.
That's reasonable, I agree that introducing macros here would not be beneficial to the maintainability of the code.
Since you don't see any code size regressions I think it's fine to go ahead as is.
LGTM from my side although my knowledge of that part of the code is still fairly limited :) |
Nice improvement, LGTM |
Add, substract, mul, mod, and increment/decrement operators are optimized. JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg zherczeg.u-szeged@partner.samsung.com
62bf8e9
to
d882709
Compare
LGTM |
Nice performance improvement. Binary size unchanged.