Skip to content

Use CESU-8 lookup table to improve lit_get_unicode_char_size_by_utf8_… #793

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
Closed
wants to merge 1 commit into from
Closed

Use CESU-8 lookup table to improve lit_get_unicode_char_size_by_utf8_… #793

wants to merge 1 commit into from

Conversation

huxinx
Copy link
Contributor

@huxinx huxinx commented Dec 29, 2015

JerryScript-DCO-1.0-Signed-off-by: Xin Hu Xin.A.Hu@intel.com

Run ./tools/run-perf-test.sh 10 times,
OS, ubuntu 15.04, 32 bit
CPU, Intel(R) Celeron(R) CPU N2820 @ 2.13GHz, 2 core

Benchmark RSS
(+ is better)		 Perf
(+ is better)
3d-cube.js 116-> 116 (0.000) 1.092->1.08089 (1.017)
access-binary-trees.js 88-> 88 (0.000) 0.660444->0.659556 (0.134)
access-fannkuch.js 44-> 44 (0.000) 3.512->3.50133 (0.304)
bitops-3bit-bits-in-byte.js 36-> 32 (11.111) 0.910667->0.916889 (-0.683)
bitops-bits-in-byte.js 32-> 36 (-12.500) 1.19689->1.20578 (-0.743)
bitops-bitwise-and.js 36-> 36 (0.000) 1.27956->1.27111 (0.660)
controlflow-recursive.js 244-> 244 (0.000) 0.56->0.570667 (-1.905)
crypto-aes.js 128-> 132 (-3.125) 2.31289->2.22622 (3.747)
crypto-md5.js 192-> 192 (0.000) 12.3418->10.3098 (16.464)
crypto-sha1.js 140-> 140 (0.000) 5.52311->4.67778 (15.305)
date-format-xparb.js 76-> 76 (0.000) 0.640444->0.629333 (1.735)
math-cordic.js 44-> 40 (9.091) 1.32267->1.29422 (2.151)
math-spectral-norm.js 44-> 44 (0.000) 0.796->0.780444 (1.954)
string-base64.js 168-> 172 (-2.381) 97.4924->89.3093 (8.394)
string-fasta.js 52-> 56 (-7.692) 2.27644->2.14844 (5.623)
Geometric mean: RSS reduction: -0.221% Speed up: 3.7729%

Tue Dec 29 06:36:58 EST 2015

In lit_get_unicode_char_size_by_utf8_first_byte(), use a lookup table to get the cesu-8 length.
Then there is no if/else branch in lit_get_unicode_char_size_by_utf8_first_byte(),
and it gets chance to be inlined.
This lookup table is pretty small, which should not affect memory a lot.

zherczeg
zherczeg reviewed Dec 29, 2015
View reviewed changes
jerry-core/lit/lit-strings.cpp
@@ -757,6 +757,15 @@ lit_utf8_string_code_unit_at (const lit_utf8_byte_t *utf8_buf_p, /**< utf-8 stri
return code_unit;
} /* lit_utf8_string_code_unit_at */

/* CESU-8 number of bytes occupied lookup table */
const lit_utf8_size_t table[]
Copy link
Member

Choose a reason for hiding this comment

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

Seems OK, but I would reduce the table size to uint8 from uint32 to reduce the table size from 64 to 16 bytes (the values are small anyway). Also the table should be aligned to 16 bytes.

Copy link
Contributor

Choose a reason for hiding this comment

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

@zherczeg, why do we need the table alignment?

@huxinx
Copy link
Contributor Author

huxinx commented Dec 29, 2015

JerryScript-DCO-1.0-Signed-off-by: Xin Hu Xin.A.Hu@intel.com

Run ./tools/run-perf-test.sh 10 times,
OS, ubuntu 15.04, 32 bit
CPU, Intel(R) Celeron(R) CPU N2820 @ 2.13GHz, 2 core

Benchmark RSS
(+ is better)		 Perf
(+ is better)
3d-cube.js 120-> 116 (3.333) 1.092->1.07244 (1.791)
access-binary-trees.js 88-> 88 (0.000) 0.66->0.661778 (-0.269)
access-fannkuch.js 48-> 48 (0.000) 3.51644->3.50222 (0.404)
bitops-3bit-bits-in-byte.js 36-> 36 (0.000) 0.911111->0.915556 (-0.488)
bitops-bits-in-byte.js 36-> 36 (0.000) 1.19556->1.23778 (-3.531)
bitops-bitwise-and.js 36-> 40 (-11.111) 1.27867->1.29556 (-1.321)
controlflow-recursive.js 244-> 244 (0.000) 0.56-> 0.576 (-2.857)
crypto-aes.js 128-> 128 (0.000) 2.31378->2.21333 (4.341)
crypto-md5.js 192-> 192 (0.000) 12.3436->10.3836 (15.879)
crypto-sha1.js 140-> 136 (2.857) 5.52356-> 4.72 (14.548)
date-format-xparb.js 76-> 76 (0.000) 0.639556-> 0.628 (1.807)
math-cordic.js 44-> 44 (0.000) 1.32133-> 1.308 (1.009)
math-spectral-norm.js 40-> 40 (0.000) 0.792889->0.780889 (1.513)
string-base64.js 168-> 168 (0.000) 97.4809->86.1049 (11.670)
string-fasta.js 56-> 52 (7.143) 2.27333->2.16178 (4.907)
Geometric mean: RSS reduction: 0.2107% Speed up: 3.4788%

Tue Dec 29 12:23:25 EST 2015

To replace uint32 with uint8, align to 16 bytes. Done.

@egavrin egavrin added the enhancement An improvement label Dec 29, 2015
@huxinx
Use CESU-8 lookup table to improve lit_get_unicode_char_size_by_utf8_…
63135ca 
…first_byte performance

JerryScript-DCO-1.0-Signed-off-by: Xin Hu Xin.A.Hu@intel.com
zherczeg
zherczeg reviewed Dec 30, 2015
View reviewed changes
jerry-core/lit/lit-strings.cpp
(first_byte >> 4) == 3 || (first_byte >> 4) == 4 ||
(first_byte >> 4) == 5 || (first_byte >> 4) == 6 ||
(first_byte >> 4) == 7 || (first_byte >> 4) == 12 ||
(first_byte >> 4) == 13 || (first_byte >> 4) == 14));
Copy link
Member

Choose a reason for hiding this comment

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

I have no objection against big asserts, but in this case wouldn't be better to use (first_byte >> 4) <= 7 ? This is just a note, so if others prefer the current one, it is ok for me.

@huxinx huxinx mentioned this pull request Dec 30, 2015
Closed
@egavrin
Copy link
Contributor

egavrin commented Dec 31, 2015

Wed Jan 21 23:57:18 UTC 1970
RPi2

Benchmark RSS
(+ is better)		 Perf
(+ is better)
3d-cube.js 112-> 112 (0.000) 3.06533->3.07067 (-0.174)
access-binary-trees.js 80-> 80 (0.000) 1.87867->1.88933 (-0.567)
access-fannkuch.js 40-> 40 (0.000) 9.12267->9.04933 (0.804)
access-nbody.js 48-> 48 (0.000) 4.228->4.20533 (0.536)
bitops-3bit-bits-in-byte.js 32-> 32 (0.000) 2.35467->2.36933 (-0.623)
bitops-bits-in-byte.js 32-> 32 (0.000) 3.128->3.11067 (0.554)
bitops-bitwise-and.js 28-> 28 (0.000) 3.51467-> 3.54 (-0.721)
bitops-nsieve-bits.js 156-> 156 (0.000) 27.612->27.6987 (-0.314)
controlflow-recursive.js 236-> 236 (0.000) 1.61467-> 1.636 (-1.321)
crypto-aes.js 120-> 120 (0.000) 5.44267->5.49333 (-0.931)
crypto-md5.js 184-> 184 (0.000) 29.3173->31.6333 (-7.900)
crypto-sha1.js 132-> 132 (0.000) 13.2693->14.1507 (-6.642)
date-format-xparb.js 72-> 72 (0.000) 1.65867->1.66533 (-0.402)
math-cordic.js 40-> 40 (0.000) 3.38133->3.39067 (-0.276)
math-partial-sums.js 36-> 36 (0.000) 1.97467->1.97733 (-0.135)
math-spectral-norm.js 40-> 40 (0.000) 2.112->2.12933 (-0.821)
string-base64.js 168-> 188 (-11.905) 250.192->263.78 (-5.431)
string-fasta.js 48-> 48 (0.000) 5.48533-> 5.556 (-1.288)
Geometric mean: RSS reduction: -0.627% Speed up: -1.397%

ruben-ayrapetyan
ruben-ayrapetyan reviewed Dec 31, 2015
View reviewed changes
jerry-core/lit/lit-strings.cpp
@@ -757,6 +757,15 @@ lit_utf8_string_code_unit_at (const lit_utf8_byte_t *utf8_buf_p, /**< utf-8 stri
return code_unit;
} /* lit_utf8_string_code_unit_at */

/* CESU-8 number of bytes occupied lookup table */
const __attribute__ ((aligned (CESU_8_TABLE_MEM_ALIGNMENT))) lit_utf8_byte_t table[]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need the alignment attribute?

@zherczeg
Copy link
Member

I think this patch has a new version without table. Anyway, alignment improves cache locality (ensures the table consumes only one page).

@ruben-ayrapetyan
Copy link
Contributor

@zherczeg, I see. Thanks for explanation.

Then its better to explain the reason in comments.

@sand1k sand1k removed their assignment Jan 11, 2016
@sand1k
Copy link
Contributor

sand1k commented Jan 11, 2016

Let's close this PR.

@huxinx huxinx closed this Jan 12, 2016
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
Projects
None yet
Milestone
Engine optimization & enhancement
Development

Successfully merging this pull request may close these issues.
5 participants
@huxinx @egavrin @zherczeg @ruben-ayrapetyan @sand1k