Skip to content

Reduce memory consumption of property names. #1445

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
Nov 25, 2016
Merged

Reduce memory consumption of property names. #1445

merged 1 commit into from
Nov 25, 2016

Conversation

@zherczeg
Copy link
Member

@zherczeg zherczeg commented Nov 24, 2016

Large memory improvements. E.g. bitops-nsieve-bits.js 28%, crypto-sha1.js 26%, crypto-md5.js 23%, ...

Binary size is unchanged (152544 bytes), however unlikely is that with such a big patch.

Performance is decreased because of the increased complexity by 1%. Most tests are not changed much, but bitops-bitwise-and.js is decreased by 9%.

Benchmark Heap (bytes) RSS (bytes) Perf (sec)
3d-cube.js 16376 -> 16376 : 0.000% 57344 -> 57344 : 0.000% 0.869 -> 0.858 : +1.230%
3d-raytrace.js 160872 -> 128496 : +20.125% 212992 -> 176128 : +17.308% 1.050 -> 1.042 : +0.775%
access-binary-trees.js 32760 -> 32760 : 0.000% 53248 -> 53248 : 0.000% 0.567 -> 0.574 : -1.258%
access-fannkuch.js 5624 -> 5376 : +4.410% 20480 -> 20480 : 0.000% 2.181 -> 2.203 : -0.972%
access-nbody.js 6672 -> 6488 : +2.758% 28672 -> 28672 : 0.000% 1.069 -> 1.081 : -1.207%
bitops-3bit-bits-in-byte.js 928 -> 920 : +0.862% 16384 -> 16384 : 0.000% 0.588 -> 0.588 : +0.084%
bitops-bits-in-byte.js 880 -> 872 : +0.909% 16384 -> 16384 : 0.000% 0.871 -> 0.875 : -0.465%
bitops-bitwise-and.js 568 -> 560 : +1.408% 16384 -> 16384 : 0.000% 1.053 -> 1.153 : -9.469%
bitops-nsieve-bits.js 138536 -> 98528 : +28.879% 155648 -> 114688 : +26.316% 1.418 -> 1.389 : +2.001%
controlflow-recursive.js 912 -> 904 : +0.877% 81920 -> 81920 : 0.000% 0.393 -> 0.398 : -1.295%
crypto-aes.js 49184 -> 40944 : +16.753% 81920 -> 73728 : +10.000% 0.953 -> 0.962 : -1.019%
crypto-md5.js 139256 -> 106488 : +23.531% 163840 -> 131072 : +20.000% 0.666 -> 0.667 : -0.135%
crypto-sha1.js 90104 -> 66576 : +26.112% 110592 -> 86016 : +22.222% 0.654 -> 0.657 : -0.394%
date-format-tofte.js 16376 -> 16376 : 0.000% 40960 -> 40960 : 0.000% 0.747 -> 0.743 : +0.521%
date-format-xparb.js 16376 -> 16384 : -0.049% 40960 -> 40960 : 0.000% 0.401 -> 0.413 : -3.050%
math-cordic.js 2096 -> 1992 : +4.962% 20480 -> 16384 : +20.000% 1.312 -> 1.312 : +0.000%
math-partial-sums.js 1432 -> 1424 : +0.559% 16384 -> 16384 : 0.000% 0.730 -> 0.769 : -5.359%
math-spectral-norm.js 8184 -> 6536 : +20.137% 24576 -> 24576 : 0.000% 0.580 -> 0.590 : -1.647%
string-base64.js 92008 -> 90976 : +1.122% 143360 -> 135168 : +5.714% 1.559 -> 1.564 : -0.309%
string-fasta.js 16376 -> 16376 : 0.000% 32768 -> 32768 : 0.000% 1.211 -> 1.243 : -2.586%
Geometric mean: +8.267% +6.561% -1.199%

@zherczeg zherczeg force-pushed the string_hashing branch 4 times, most recently from e146594 to e11ae2e Compare November 25, 2016 09:03
@LaszloLango LaszloLango added enhancement An improvement memory consumption Affects memory consumption labels Nov 25, 2016
LaszloLango
LaszloLango reviewed Nov 25, 2016
View reviewed changes
jerry-core/ecma/base/ecma-globals.h Outdated
lit_magic_string_id_t magic_string_id; /**< identifier of a magic string */
lit_magic_string_ex_id_t magic_string_ex_id; /**< identifier of an external magic string */
uint32_t magic_string_id; /**< identifier of a magic string */
uint32_t magic_string_ex_id; /**< identifier of an external magic string */
Copy link
Contributor

@LaszloLango LaszloLango Nov 25, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Why did you change the type of magic_string_id and magic_string_ex_id?

Copy link
Member Author

@zherczeg zherczeg Nov 25, 2016

Choose a reason for hiding this comment

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

The problem was that their storage size was unknown before. Now uint32_number, magic_string_id, and magic_string_ex_id uses the same storage type (common_uint32_field) and this opens up several optimizations.

Copy link
Contributor

@LaszloLango LaszloLango Nov 25, 2016

Choose a reason for hiding this comment

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

The comment should refer to to their real types. Please update the comments.

@zherczeg zherczeg force-pushed the string_hashing branch 2 times, most recently from e11ae2e to 7c08132 Compare November 25, 2016 13:45
@zherczeg
Reduce memory consumption of property names.
6d67b8d 
Property names were always required a string reference which consumed
a large amount of memory for arrays. This patch reduces this consumption
by directly storing the value part of certain strings.

JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg zherczeg.u-szeged@partner.samsung.com
LaszloLango
LaszloLango approved these changes Nov 25, 2016
View reviewed changes
Copy link
Contributor

@LaszloLango LaszloLango left a comment

Choose a reason for hiding this comment

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

LGTM

dbatyai
dbatyai approved these changes Nov 25, 2016
View reviewed changes
Copy link
Member

@dbatyai dbatyai left a comment

Choose a reason for hiding this comment

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

LGTM

@zherczeg zherczeg merged commit 6d67b8d into jerryscript-project:master Nov 25, 2016
@zherczeg zherczeg deleted the string_hashing branch November 25, 2016 14:39
@janjongboom janjongboom mentioned this pull request Nov 28, 2016
Merged
@tilmannOSG
Copy link

tilmannOSG commented Dec 5, 2016

@zherczeg Nice, great improvement!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LaszloLango LaszloLango LaszloLango approved these changes

@dbatyai dbatyai dbatyai approved these changes

Assignees

No one assigned

Labels

enhancement An improvement memory consumption Affects memory consumption

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@zherczeg @tilmannOSG @LaszloLango @dbatyai