From fc5bb4b10e7cbcde926cde272b6533e0747391fc Mon Sep 17 00:00:00 2001 From: sasensi Date: Tue, 2 Oct 2018 14:09:34 +0200 Subject: [PATCH 1/3] Fix #1493 Path#add crashes whith 1000000 segments --- src/path/Path.js | 6 ++++-- test/tests/Path.js | 10 ++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/path/Path.js b/src/path/Path.js index 42eaa34e..81bcfa05 100644 --- a/src/path/Path.js +++ b/src/path/Path.js @@ -404,8 +404,10 @@ var Path = PathItem.extend(/** @lends Path# */{ this._updateSelection(segment, 0, segment._selection); } if (append) { - // Append them all at the end by using push - segments.push.apply(segments, segs); + // Append them all at the end. + // Use a loop as it is the best way to handle big arrays (see #1493) + for (var i = 0; i < segs.length; i++) + segments.push(segs[i]); } else { // Insert somewhere else segments.splice.apply(segments, [index, 0].concat(segs)); diff --git a/test/tests/Path.js b/test/tests/Path.js index 91a1eacf..70cd401e 100644 --- a/test/tests/Path.js +++ b/test/tests/Path.js @@ -611,3 +611,13 @@ test('Path#arcTo(from, through, to); where from, through and to all share the sa } equals(error != null, true, 'We expect this arcTo() command to throw an error'); }); + +test('#1493 Path#add with 1000000 segments', function() { + var path = new Path(); + for (var i=0; i<1000000; i++) + { + path.add(new Point(0,0)); + } + path.clone(); + expect(0); +}); From f6735426402eb5800a22a09b4d0ee562e3d09a39 Mon Sep 17 00:00:00 2001 From: sasensi Date: Tue, 2 Oct 2018 19:28:35 +0200 Subject: [PATCH 2/3] Refactor code to fit style rules --- src/path/Path.js | 3 ++- test/tests/Path.js | 5 ++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/path/Path.js b/src/path/Path.js index 81bcfa05..c270a2ca 100644 --- a/src/path/Path.js +++ b/src/path/Path.js @@ -406,8 +406,9 @@ var Path = PathItem.extend(/** @lends Path# */{ if (append) { // Append them all at the end. // Use a loop as it is the best way to handle big arrays (see #1493) - for (var i = 0; i < segs.length; i++) + for (var i = 0, l = segs.length; i < l; i++) { segments.push(segs[i]); + } } else { // Insert somewhere else segments.splice.apply(segments, [index, 0].concat(segs)); diff --git a/test/tests/Path.js b/test/tests/Path.js index 70cd401e..ae190b10 100644 --- a/test/tests/Path.js +++ b/test/tests/Path.js @@ -614,9 +614,8 @@ test('Path#arcTo(from, through, to); where from, through and to all share the sa test('#1493 Path#add with 1000000 segments', function() { var path = new Path(); - for (var i=0; i<1000000; i++) - { - path.add(new Point(0,0)); + for (var i = 0; i < 1000000; i++) { + path.add(new Point(0, 0)); } path.clone(); expect(0); From d12b99e252114a204c7d000f9045e9f8cc864380 Mon Sep 17 00:00:00 2001 From: sasensi Date: Wed, 3 Oct 2018 08:59:36 +0200 Subject: [PATCH 3/3] Improve Path#add performance with big arrays --- src/path/Path.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/path/Path.js b/src/path/Path.js index c270a2ca..c516176b 100644 --- a/src/path/Path.js +++ b/src/path/Path.js @@ -405,9 +405,13 @@ var Path = PathItem.extend(/** @lends Path# */{ } if (append) { // Append them all at the end. - // Use a loop as it is the best way to handle big arrays (see #1493) - for (var i = 0, l = segs.length; i < l; i++) { - segments.push(segs[i]); + // Use a loop as the best way to handle big arrays (see #1493). + // Set future array length before the loop for better performances. + var originalLength = segments.length; + var offsetLength = segs.length; + segments.length += offsetLength; + for (var i = 0; i < offsetLength; i++) { + segments[originalLength + i] = segs[i]; } } else { // Insert somewhere else