fix a bug in linked_list memory allocation #720

jiangzidong · 2015-11-16T10:56:51Z

The correct heap block for linked_list should be like:

first chunk

non-first chunk

| jsp_mm_header | linked_list_chunk_header | data |

However the linked_list_block_size() shows

first chunk

| jsp_mm_header | linked_list_header | data |

non-first chunk

it is wrong
it will cause more waste and fragment in mem_heap.

After the patch, the performance and rss data are as follows:

in ubuntu14.04 32bit 3.16.0-30-generic, Intel(R) Core(TM) i7-4770K CPU @ 3.50GHz

data is updated after @ruben-ayrapetyan's fix

jzd@jzd32:~/jerryscript$ ./tools/run-perf-test.sh compare_engine/jerry.orig compare_engine/jerry.ll 5 5000 sunspider-1.0.2/

Benchmark	RSS (+ is better)	Perf (+ is better)
3d-cube.js	136-> 132 (2.941)	0.352-> 0.352 (0.000)
access-binary-trees.js	88-> 88 (0.000)	0.164-> 0.164 (0.000)
access-fannkuch.js	48-> 48 (0.000)	0.916->0.9184 (-0.262)
bitops-3bit-bits-in-byte.js	44-> 40 (9.091)	0.276-> 0.276 (0.000)
bitops-bits-in-byte.js	40-> 40 (0.000)	0.3616->0.3616 (0.000)
bitops-bitwise-and.js	40-> 40 (0.000)	0.348->0.3496 (-0.460)
controlflow-recursive.js	244-> 240 (1.639)	0.1536->0.1536 (0.000)
crypto-aes.js	160-> 156 (2.500)	0.6176-> 0.616 (0.259)
crypto-md5.js	208-> 204 (1.923)	3.7424->3.7448 (-0.064)
crypto-sha1.js	144-> 148 (-2.778)	1.616-> 1.62 (-0.248)
date-format-tofte.js	116-> 108 (6.897)	0.396->0.3928 (0.808)
date-format-xparb.js	96-> 96 (0.000)	0.188-> 0.188 (0.000)
math-cordic.js	48-> 48 (0.000)	0.3856-> 0.384 (0.415)
math-spectral-norm.js	48-> 44 (8.333)	0.2-> 0.2 (0.000)
string-base64.js	200-> 204 (-2.000)	28.6832->28.6928 (-0.033)
string-fasta.js	60-> 60 (0.000)	0.624-> 0.624 (0.000)
Geometric mean:	RSS reduction: 1.8434%	Speed up: 0.0264%

the RSS data is coarse and not stable because its minimal unit is 4k.

We can see the mem-stats data for more details.

jzd@jzd32:~/jerryscript$ ./compare_engine/jerry.orig.mem --mem-stats sunspider-1.0.2/3d-cube.js
Heap stats:
  Heap size = 260096 bytes
  Chunk size = 64 bytes
  Allocated chunks count = 0
  Allocated = 0 bytes
  Waste = 0 bytes
  Peak allocated chunks count = 655
  Peak allocated = 41080 bytes
  Peak waste = 4181 bytes

jzd@jzd32:~/jerryscript$ ./compare_engine/jerry.ll.mem --mem-stats sunspider-1.0.2/3d-cube.js
Heap stats:
  Heap size = 260096 bytes
  Chunk size = 64 bytes
  Allocated chunks count = 0
  Allocated = 0 bytes
  Waste = 0 bytes
  Peak allocated chunks count = 646
  Peak allocated = 41080 bytes
  Peak waste = 725 bytes

for 3d-cube:
Peak allocated chunks count: 655 -> 646
Peak waste: 4181 -> 725

jzd@jzd32:~/jerryscript$ ./compare_engine/jerry.orig.mem --mem-stats sunspider-1.0.2/access-fannkuch.js
Heap stats:
  Heap size = 260096 bytes
  Chunk size = 64 bytes
  Allocated chunks count = 0
  Allocated = 0 bytes
  Waste = 0 bytes
  Peak allocated chunks count = 104
  Peak allocated = 5967 bytes
  Peak waste = 689 bytes


jzd@jzd32:~/jerryscript$ ./compare_engine/jerry.ll.mem --mem-stats sunspider-1.0.2/access-fannkuch.js
Heap stats:
  Heap size = 260096 bytes
  Chunk size = 64 bytes
  Allocated chunks count = 0
  Allocated = 0 bytes
  Waste = 0 bytes
  Peak allocated chunks count = 99
  Peak allocated = 6099 bytes
  Peak waste = 237 bytes

for access-fannkuch.js
Peak allocated chunks count: 104 -> 99
Peak waste = 689 -> 237

ruben-ayrapetyan · 2015-11-16T13:06:23Z

@jiangzidong, great improvement!

Raspberry PI 2 measurements

Benchmark	RSS (+ is better)	Perf (+ is better)
3d-cube.js	128-> 124 (3.125)	3.80867->3.81967 (-0.289)
access-binary-trees.js	88-> 84 (4.545)	2.00033-> 2.017 (-0.833)
access-fannkuch.js	48-> 48 (0.000)	9.42167->9.37867 (0.456)
access-nbody.js	64-> 64 (0.000)	4.543->4.54667 (-0.081)
bitops-3bit-bits-in-byte.js	40-> 40 (0.000)	2.74633->2.74333 (0.109)
bitops-bits-in-byte.js	40-> 40 (0.000)	3.811-> 3.761 (1.312)
bitops-bitwise-and.js	36-> 36 (0.000)	4.14767-> 4.18 (-0.779)
controlflow-recursive.js	220-> 220 (0.000)	1.846-> 1.896 (-2.709)
date-format-xparb.js	96-> 96 (0.000)	2.27367-> 2.166 (4.736)
math-cordic.js	48-> 48 (0.000)	4.027->4.07033 (-1.076)
math-partial-sums.js	44-> 40 (9.091)	2.297-> 2.297 (0.000)
math-spectral-norm.js	48-> 44 (8.333)	2.35633->2.34367 (0.537)
string-fasta.js	56-> 56 (0.000)	5.82433->5.87233 (-0.824)
Geometric mean:	RSS reduction: 1.9846%	Speed up: 0.0569%

Also, the change revealed an issue in `linked_list_switch_to_next_elem`:

diff --git a/jerry-core/parser/js/collections/linked-list.cpp b/jerry-core/parser/js/collections/linked-list.cpp
index d0448f2..b293832 100644
--- a/jerry-core/parser/js/collections/linked-list.cpp
+++ b/jerry-core/parser/js/collections/linked-list.cpp
@@ -152,7 +152,7 @@ linked_list_switch_to_next_elem (linked_list_header *header_p, /**< list header
   linked_list_chunk_header *chunk_header_p = *in_out_chunk_header_p;

   const size_t element_size = header_p->element_size;
-  const bool is_first_chunk = ((linked_list_chunk_header *) header_p + 1u == chunk_header_p);
+  const bool is_first_chunk = ((linked_list_chunk_header *) (header_p + 1u) == chunk_header_p);

   JERRY_ASSERT (raw_elem_ptr_p + element_size
                 <= (uint8_t *) (chunk_header_p + 1u) + linked_list_block_size (is_first_chunk));

Could you, please, add this fix also?

With the fix measurements on Raspberry PI 2 are the following:

Benchmark	RSS (+ is better)	Perf (+ is better)
3d-cube.js	128-> 124 (3.125)	3.824->3.84267 (-0.488)
access-binary-trees.js	88-> 84 (4.545)	1.96067-> 2 (-2.006)
access-fannkuch.js	48-> 48 (0.000)	9.402->9.40233 (-0.004)
access-nbody.js	64-> 64 (0.000)	4.54167->4.51433 (0.602)
bitops-3bit-bits-in-byte.js	40-> 40 (0.000)	2.755->2.77933 (-0.883)
bitops-bits-in-byte.js	40-> 40 (0.000)	3.786->3.83333 (-1.250)
bitops-bitwise-and.js	36-> 36 (0.000)	4.16933->4.10467 (1.551)
controlflow-recursive.js	220-> 220 (0.000)	1.86067->1.89433 (-1.809)
date-format-xparb.js	96-> 96 (0.000)	2.26467->2.27567 (-0.486)
math-cordic.js	48-> 48 (0.000)	4.06933-> 4.07 (-0.016)
math-partial-sums.js	44-> 40 (9.091)	2.37533->2.35667 (0.786)
math-spectral-norm.js	48-> 44 (8.333)	2.39833-> 2.356 (1.765)
string-fasta.js	56-> 56 (0.000)	5.84233->5.78367 (1.004)
Geometric mean:	RSS reduction: 1.9846%	Speed up: -0.088%

egavrin · 2015-11-16T14:16:31Z

@jiangzidong nice catch!

jiangzidong · 2015-11-16T14:41:16Z

@ruben-ayrapetyan Thanks for your reminding.
@egavrin How can we run test262 in local environment? I tried the run-test-suite-test262.sh, but even the original jerryscript cannot have 100% pass rate.

egavrin · 2015-11-19T12:05:11Z

@jiangzidong please add Related issue: #720 to commit message.

and Ruben Ayrapetyan fix a bug in linked_list_switch_to_next_elem Related issue: jerryscript-project#720 JerryScript-DCO-1.0-Signed-off-by: Zidong Jiang [email protected]

jiangzidong · 2015-11-19T22:49:36Z

@egavrin ok, done

ruben-ayrapetyan · 2015-11-20T10:20:38Z

Looks good to me

egavrin · 2015-11-26T17:15:44Z

@galpeter please review this PR. I think we may merge it soon.

zherczeg · 2015-11-27T10:51:39Z

LGTM

and Ruben Ayrapetyan fix a bug in linked_list_switch_to_next_elem Related issue: #720 JerryScript-DCO-1.0-Signed-off-by: Zidong Jiang [email protected]

egavrin · 2015-12-02T09:18:07Z

Merged 3d286b4

and Ruben Ayrapetyan fix a bug in linked_list_switch_to_next_elem Related issue: #720 JerryScript-DCO-1.0-Signed-off-by: Zidong Jiang [email protected]

and Ruben Ayrapetyan fix a bug in linked_list_switch_to_next_elem Related issue: jerryscript-project#720 JerryScript-DCO-1.0-Signed-off-by: Zidong Jiang [email protected]

egavrin added this to the Engine optimization & enhancement milestone Nov 16, 2015

egavrin added bug Undesired behaviour enhancement An improvement parser Related to the JavaScript parser labels Nov 16, 2015

jiangzidong force-pushed the linked_list branch from 5d14eed to 7bd9393 Compare November 16, 2015 12:24

jiangzidong force-pushed the linked_list branch from 7bd9393 to 66b2e04 Compare November 16, 2015 13:59

egavrin closed this Nov 19, 2015

egavrin reopened this Nov 19, 2015

jiangzidong force-pushed the linked_list branch from 66b2e04 to 08e5558 Compare November 19, 2015 12:31

Fix bug in linked_list_block_size().

08e5558

and Ruben Ayrapetyan fix a bug in linked_list_switch_to_next_elem Related issue: jerryscript-project#720 JerryScript-DCO-1.0-Signed-off-by: Zidong Jiang [email protected]

egavrin assigned galpeter Nov 26, 2015

galpeter removed their assignment Nov 27, 2015

egavrin pushed a commit that referenced this pull request Dec 2, 2015

Fix bug in linked_list_block_size().

3d286b4

and Ruben Ayrapetyan fix a bug in linked_list_switch_to_next_elem Related issue: #720 JerryScript-DCO-1.0-Signed-off-by: Zidong Jiang [email protected]

egavrin closed this Dec 2, 2015

sand1k pushed a commit that referenced this pull request Dec 24, 2015

Fix bug in linked_list_block_size().

7e845a2

and Ruben Ayrapetyan fix a bug in linked_list_switch_to_next_elem Related issue: #720 JerryScript-DCO-1.0-Signed-off-by: Zidong Jiang [email protected]

zherczeg mentioned this pull request Jan 19, 2017

Nominating Zidong Jiang (@jiangzidong) to become a JerryScript committer #1535

Closed

jiangzidong deleted the linked_list branch February 8, 2017 02:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix a bug in linked_list memory allocation #720

fix a bug in linked_list memory allocation #720

Uh oh!

jiangzidong commented Nov 16, 2015

Uh oh!

ruben-ayrapetyan commented Nov 16, 2015

Uh oh!

egavrin commented Nov 16, 2015

Uh oh!

jiangzidong commented Nov 16, 2015

Uh oh!

egavrin commented Nov 19, 2015

Uh oh!

jiangzidong commented Nov 19, 2015

Uh oh!

ruben-ayrapetyan commented Nov 20, 2015

Uh oh!

egavrin commented Nov 26, 2015

Uh oh!

zherczeg commented Nov 27, 2015

Uh oh!

egavrin commented Dec 2, 2015

Uh oh!

Uh oh!

fix a bug in linked_list memory allocation #720

fix a bug in linked_list memory allocation #720

Uh oh!

Conversation

jiangzidong commented Nov 16, 2015

Uh oh!

ruben-ayrapetyan commented Nov 16, 2015

Uh oh!

egavrin commented Nov 16, 2015

Uh oh!

jiangzidong commented Nov 16, 2015

Uh oh!

egavrin commented Nov 19, 2015

Uh oh!

jiangzidong commented Nov 19, 2015

Uh oh!

ruben-ayrapetyan commented Nov 20, 2015

Uh oh!

egavrin commented Nov 26, 2015

Uh oh!

zherczeg commented Nov 27, 2015

Uh oh!

egavrin commented Dec 2, 2015

Uh oh!

Uh oh!