Remove ECMA_STRING_CONTAINER_CONCATENATION type from ecma_string

[LaszloLango]

JerryScript-DCO-1.0-Signed-off-by: Kristof Kosztyo kkosztyo.u-szeged@partner.samsung.com

Base rev: 6697523

Benchmark	RSS (+ is better)	Perf (+ is better)
3d-cube.js	152-> 156 (-2.632)	0.3486->0.3409 (2.209)
access-binary-trees.js	96-> 100 (-4.167)	0.2488->0.2504 (-0.643)
access-fannkuch.js	52-> 52 (0.000)	0.9333->0.9112 (2.368)
access-nbody.js	72-> 68 (5.556)	0.4593->0.4596 (-0.065)
bitops-3bit-bits-in-byte.js	44-> 44 (0.000)	0.319->0.3091 (3.103)
bitops-bits-in-byte.js	36-> 36 (0.000)	0.4385->0.4246 (3.170)
bitops-bitwise-and.js	44-> 40 (9.091)	0.364->0.3608 (0.879)
controlflow-recursive.js	288-> 292 (-1.389)	0.2875->0.2816 (2.052)
crypto-aes.js	224-> 164 (26.786)	2.2872->0.6263 (72.617)
date-format-xparb.js	108-> 108 (0.000)	0.2919->0.2522 (13.600)
math-cordic.js	48-> 52 (-8.333)	0.4142->0.3866 (6.663)
math-partial-sums.js	44-> 44 (0.000)	0.2331->0.2331 (0.000)
math-spectral-norm.js	56-> 56 (0.000)	0.2981->0.2934 (1.577)
string-fasta.js	68-> 64 (5.882)	2.9977->1.4992 (49.988)
Geometric mean:	RSS reduction: 2.5681%	Speed up: 15.4631%

[LaszloLango]

See also: #548
before: 1m18.623s
after: 0m0.042s

[ruben-ayrapetyan]

@LaszloLango, could you, please, describe memory footprint changes related to the patch? Also, could you, please, provide memory footprints comparision on some "concatenation-only" tests to make changes, related to the patch, more clear?

[LaszloLango]

@ruben-ayrapetyan,

Benchmark	RSS (+ is better)	Perf (+ is better)
issue_548.js	164-> 48 (70.732)	78.645-> 0.034 (99.957)
Geometric mean:	RSS reduction: 70.7317%	Speed up: 99.9568%

[sand1k]

Measured on RaspberryPi:

Benchmark	RSS (+ is better)	Perf (+ is better)
3d-cube.js	140-> 132 (5.714)	3.696->3.65867 (1.010)
access-binary-trees.js	88-> 88 (0.000)	2.78533->2.73067 (1.962)
access-fannkuch.js	40-> 40 (0.000)	9.82667->9.86933 (-0.434)
access-nbody.js	64-> 64 (0.000)	4.664->4.62933 (0.743)
bitops-3bit-bits-in-byte.js	36-> 36 (0.000)	3.31067-> 3.244 (2.014)
bitops-bits-in-byte.js	36-> 36 (0.000)	4.52533->4.42133 (2.298)
bitops-bitwise-and.js	32-> 32 (0.000)	4.14933->4.12933 (0.482)
controlflow-recursive.js	220-> 220 (0.000)	3.30267->3.20133 (3.068)
date-format-xparb.js	92-> 100 (-8.696)	3.21333->2.35733 (26.639)
math-cordic.js	48-> 48 (0.000)	4.92133-> 4.848 (1.490)
math-partial-sums.js	40-> 40 (0.000)	2.576->2.56267 (0.517)
math-spectral-norm.js	52-> 52 (0.000)	3.23467->3.20933 (0.783)
string-fasta.js	56-> 56 (0.000)	27.2547->14.9587 (45.115)
Geometric mean:	RSS reduction: -0.189%	Speed up: 7.7627%

[galpeter]

It's strange that there are different number of files measured on intel/rpi

[sand1k]

@galpeter, jerry fails on crypto-aes.js (exit code is 1), so we don't use it for testing.

[LaszloLango]

@sand1k, thanks for the RPi results. The crypto-aes.js does not fail for me (intel/release.linux). What cause the fail? Shouldn't we fix the sunspider tests to be able to run as much tests as possible? IMHO results on a subset of any benchmark could be misleading.

[egavrin]

Nice improvement!
Good to me.

[dbatyai]

Looks good to me.

[ruben-ayrapetyan]

@LaszloLango, could you, please, describe memory footprint changes related to the patch? Also, could you, please, provide memory footprints comparision on some "concatenation-only" tests to make changes, related to the patch, more clear?

@ruben-ayrapetyan,

Benchmark	RSS (+ is better)	Perf (+ is better)
issue_548.js	164-> 48 (70.732)	78.645-> 0.034 (99.957)
Geometric mean:	RSS reduction: 70.7317%	Speed up: 99.9568%

@LaszloLango, what do you think about the following test?

var i = '0123456789ABCDEF';

for (j = 0; j < 12; j++)
{
  i += i;
}

print (i[124]);

Please, see memory statistics for the following versions:
6697523 (base version) -> 3b0f61a (the patch)

• Heap stats:
  • Peak allocated chunks count = 1037 -> 3081
  • Peak allocated = 66292 -> 197108 bytes
  • Peak waste = 199 -> 199 bytes
• Pools stats:
  • Peak pools: 8 -> 2052
  • Peak allocated chunks: 63 -> 16416

Are there any considerations on how the regression is planned to be fixed?

[LaszloLango]

@ruben-ayrapetyan,

thanks for your comment. Both implementation have advantages and disadvantages. In the new solution, if you concatenate the exact same string, then it will use more memory than before. This usecase is less frequent. I think that because of the benchmark results.

I could suggest three solutions after your comment:

1. Roll back to 6697523
2. Use build option to switch between these two concat handling
3. Leave as is

[ruben-ayrapetyan]

In the new solution, if you concatenate the exact same string, then it will use more memory than before.

This is not the only case. Please, see the following examples:

var i = '0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF';
var j = 'abcdefghijklmnopabcdefghijklmnopabcdefghijklmnopabcdefghijklmnop';

var a = '';

for (k = 0; k < 16; k++)
{
  if (k % 4 == 0)
  {
    a += (i + j);
  }
  else if (k % 2 == 0)
  {
    i += a;
  }
  else
  {
    j += a;
  }
}

print (a[124]);

• Heap stats:
  • Peak allocated chunks count = 367 -> 2533
  • Peak allocated = 23420 -> 162044 bytes
  • Peak waste = 191 -> 191 bytes
• Pools stats:
  • Peak pools: 10 -> 1943
  • Peak allocated chunks: 73 -> 15540

var i = '01';
var j = 'ab';

var a = '';

for (k = 0; k < 16; k++)
{
  if (k % 4 == 0)
  {
    a += (i + j);
  }
  else if (k % 2 == 0)
  {
    i += a;
  }
  else
  {
    j += a;
  }
}

print (a[124]);

• Heap stats:
  • Peak allocated chunks count = 63 -> 92
  • Peak allocated = 3843 -> 5766 bytes
  • Peak waste = 191 -> 191 bytes
• Pools stats:
  • Peak pools: 10 -> 56
  • Peak allocated chunks: 75 -> 525

[ruben-ayrapetyan]

I could suggest three solutions after your comment:

1. Roll back to 6697523
2. Use build option to switch between these two concat handling
3. Leave as is

@LaszloLango, considering #635 (comment), what do you think of the fourth - to perform investigation of ways to improve / optimize concatenation representation and processing routines for various cases, producing a comprehensive solution?

[LaszloLango]

@ruben-ayrapetyan, OK. Give me some time to investigate this. I have some ideas for the issue. Could you raise a GitHub issue for this conversation?

[ruben-ayrapetyan]

@LaszloLango, #648.