Skip to content

Small performance optimizations of the interpreter. #1186

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 6, 2016
Merged

Small performance optimizations of the interpreter. #1186

merged 1 commit into from
Jul 6, 2016

Conversation

zherczeg
Copy link
Member

@zherczeg zherczeg commented Jul 5, 2016

All tests are improved. Binary size unchanged.

Benchmark Perf (sec)
3d-cube.js 0.981 -> 0.949 : +3.313%
3d-raytrace.js 1.136 -> 1.114 : +1.904%
access-binary-trees.js 0.620 -> 0.585 : +5.724%
access-fannkuch.js 2.673 -> 2.566 : +3.992%
access-nbody.js 1.153 -> 1.075 : +6.798%
bitops-3bit-bits-in-byte.js 0.584 -> 0.570 : +2.297%
bitops-bits-in-byte.js 0.861 -> 0.850 : +1.281%
bitops-bitwise-and.js 1.461 -> 1.197 : +18.099%
bitops-nsieve-bits.js 1.901 -> 1.818 : +4.368%
controlflow-recursive.js 0.434 -> 0.402 : +7.427%
crypto-aes.js 1.202 -> 1.145 : +4.744%
crypto-md5.js 0.772 -> 0.753 : +2.445%
crypto-sha1.js 0.727 -> 0.709 : +2.522%
date-format-tofte.js 0.908 -> 0.887 : +2.280%
date-format-xparb.js 0.495 -> 0.481 : +2.671%
math-cordic.js 1.434 -> 1.350 : +5.867%
math-partial-sums.js 0.812 -> 0.747 : +8.064%
math-spectral-norm.js 0.610 -> 0.596 : +2.278%
string-base64.js 2.116 -> 2.093 : +1.122%
string-fasta.js 1.915 -> 1.856 : +3.109%
Geometric mean: +4.592%

@LaszloLango LaszloLango added enhancement An improvement performance Affects performance labels Jul 5, 2016
@tilmannOSG
Copy link

tilmannOSG commented Jul 5, 2016 •
edited
Loading

Nice improvement!
Speaking as someone who is unfamiliar with the code being changed, can you please elaborate on the actual optimization in the commit message? :)

@LaszloLango
Copy link
Contributor

LGTM

dbatyai
dbatyai reviewed Jul 5, 2016
View reviewed changes
jerry-core/ecma/operations/ecma-reference.c
@@ -57,69 +59,64 @@ ecma_op_resolve_reference_base (ecma_object_t *lex_env_p, /**< starting lexical
} /* ecma_op_resolve_reference_base */

/**
* Resolve syntactic reference to ECMA-reference.
* Resolve value corresponds to reference.
Copy link
Member

@dbatyai dbatyai Jul 5, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

corresponding?

@dbatyai
Copy link
Member

dbatyai commented Jul 5, 2016

LGTM otherwise.

@zherczeg
Small performance optimizations of the interpreter.
41337db 
Short summary of the changes:
 - The ecma_reference_t is removed, and its helper functions are deleted.
 - The delete operation does not depend on ecma_reference_t anymore.
 - A new resolve function is added which returns the current value
   of a named binding in the context chain.
 - The vm_op_set_value does not perform type conversions when its arguments
   has the appropriate types.

JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg zherczeg.u-szeged@partner.samsung.com
@zherczeg
Copy link
Member Author

zherczeg commented Jul 6, 2016

Thank you for the review, I fixed the issues.

@tilmannOSG
Copy link

Excellent! Thanks for the updated commit message. LGTM from my side as well.

@zherczeg zherczeg merged commit 41337db into jerryscript-project:master Jul 6, 2016
@zherczeg zherczeg deleted the optimize_vm branch July 6, 2016 07:28
@zherczeg zherczeg mentioned this pull request Jul 20, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement An improvement performance Affects performance
Projects
None yet
Milestone
Engine optimization & enhancement
Development

Successfully merging this pull request may close these issues.
4 participants
@zherczeg @tilmannOSG @LaszloLango @dbatyai