Skip to content

Optimize ecma_string_get_array_index #1207

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

Merged
merged 1 commit into from
Jul 15, 2016

Conversation

dbatyai
Copy link
Member

@dbatyai dbatyai commented Jul 14, 2016

JerryScript-DCO-1.0-Signed-off-by: Dániel Bátyai dbatyai@inf.u-szeged.hu

@dbatyai dbatyai added the enhancement An improvement label Jul 14, 2016
@dbatyai
Copy link
Member Author

dbatyai commented Jul 14, 2016

Benchmark Perf (sec)
3d-cube.js 0.944 -> 0.944 : -0.019%
3d-raytrace.js 1.114 -> 1.117 : -0.253%
access-binary-trees.js 0.582 -> 0.583 : -0.202%
access-fannkuch.js 2.365 -> 2.361 : +0.165%
access-nbody.js 1.064 -> 1.065 : -0.119%
bitops-3bit-bits-in-byte.js 0.582 -> 0.582 : +0.005%
bitops-bits-in-byte.js 0.869 -> 0.869 : +0.001%
bitops-bitwise-and.js 0.939 -> 0.940 : -0.092%
bitops-nsieve-bits.js 1.799 -> 1.791 : +0.493%
controlflow-recursive.js 0.379 -> 0.379 : -0.028%
crypto-aes.js 1.114 -> 1.116 : -0.161%
crypto-md5.js 0.733 -> 0.734 : -0.043%
crypto-sha1.js 0.695 -> 0.696 : -0.087%
date-format-tofte.js 0.865 -> 0.867 : -0.192%
date-format-xparb.js 0.455 -> 0.454 : +0.164%
math-cordic.js 1.318 -> 1.320 : -0.174%
math-partial-sums.js 0.692 -> 0.692 : +0.034%
math-spectral-norm.js 0.584 -> 0.584 : -0.024%
string-base64.js 2.053 -> 2.055 : -0.128%
string-fasta.js 1.761 -> 1.355 : +23.066%
Geometric mean: +1.270%

Binary sizes (bytes)
35c0869:156236
ae50973:156236

@LaszloLango LaszloLango added the performance Affects performance label Jul 14, 2016
if (ECMA_STRING_GET_CONTAINER (str_p) == ECMA_STRING_CONTAINER_UINT32_IN_DESC)
uint32_t index;
const ecma_string_container_t type = ECMA_STRING_GET_CONTAINER (str_p);
if (type == ECMA_STRING_CONTAINER_UINT32_IN_DESC)
Copy link
Member

Choose a reason for hiding this comment

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

Newline before if.

@zherczeg
Copy link
Member

Nice catch btw.

@LaszloLango
Copy link
Contributor

LGTM


ecma_deref_ecma_string (to_uint32_to_string_p);
}
if (size > 10
Copy link
Member

Choose a reason for hiding this comment

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

I would put the zero check first:

if (*raw_str_p == LIT_CHAR_0) return size == 1;

if (size == 10)
{
if (index > UINT32_MAX / 10
|| raw_str_p[9] > LIT_CHAR_9 || raw_str_p[9] < LIT_CHAR_0)
Copy link
Member

Choose a reason for hiding this comment

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

Style issue: two || in one line.

JerryScript-DCO-1.0-Signed-off-by: Dániel Bátyai dbatyai@inf.u-szeged.hu
@zherczeg
Copy link
Member

LGTM

@dbatyai dbatyai merged commit 3354da3 into jerryscript-project:master Jul 15, 2016
@dbatyai dbatyai deleted the array_index_opt branch January 25, 2017 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement performance Affects performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants