Improve handling of push() with lots of items

Improves fix for #1493, should pass CI again
This commit is contained in:
Jürg Lehni 2018-10-03 16:45:38 +02:00
parent da3a36230f
commit 27b92a6007
7 changed files with 37 additions and 22 deletions

View file

@ -571,6 +571,27 @@ statics: /** @lends Base */{
}); });
}, },
/**
* Utility function for pushing a large amount of items to an array.
*/
push: function(list, items) {
var itemsLength = items.length;
// It seems for "small" amounts of items, this performs better,
// but once it reaches a certain amount, some browsers start crashing:
if (itemsLength < 4096) {
list.push.apply(list, items);
} else {
// Use a loop as the best way to handle big arrays (see #1493).
// Set new array length once before the loop for better performance.
var startLength = list.length;
list.length += itemsLength;
for (var i = 0; i < itemsLength; i++) {
list[startLength + i] = items[i];
}
}
return list;
},
/** /**
* Utility function for adding and removing items from a list of which each * Utility function for adding and removing items from a list of which each
* entry keeps a reference to its index in the list in the private _index * entry keeps a reference to its index in the list in the private _index
@ -587,14 +608,14 @@ statics: /** @lends Base */{
items[i]._index = index + i; items[i]._index = index + i;
if (append) { if (append) {
// Append them all at the end by using push // Append them all at the end by using push
list.push.apply(list, items); Base.push(list, items);
// Nothing removed, and nothing to adjust above // Nothing removed, and nothing to adjust above
return []; return [];
} else { } else {
// Insert somewhere else and/or remove // Insert somewhere else and/or remove
var args = [index, remove]; var args = [index, remove];
if (items) if (items)
args.push.apply(args, items); Base.push(args, items);
var removed = list.splice.apply(list, args); var removed = list.splice.apply(list, args);
// Erase the indices of the removed items // Erase the indices of the removed items
for (var i = 0, l = removed.length; i < l; i++) for (var i = 0, l = removed.length; i < l; i++)

View file

@ -19,7 +19,7 @@ module.exports = function(self, requireName) {
var Canvas; var Canvas;
try { try {
Canvas = require('canvas'); Canvas = require('canvas');
} catch(e) { } catch(error) {
// Remove `self.window`, so we still have the global `self` reference, // Remove `self.window`, so we still have the global `self` reference,
// but no `window` object: // but no `window` object:
// - On the browser, this corresponds to a worker context. // - On the browser, this corresponds to a worker context.

View file

@ -213,8 +213,9 @@ var CompoundPath = PathItem.extend(/** @lends CompoundPath# */{
getCurves: function() { getCurves: function() {
var children = this._children, var children = this._children,
curves = []; curves = [];
for (var i = 0, l = children.length; i < l; i++) for (var i = 0, l = children.length; i < l; i++) {
curves.push.apply(curves, children[i].getCurves()); Base.push(curves, children[i].getCurves());
}
return curves; return curves;
}, },

View file

@ -2128,7 +2128,7 @@ new function() { // Scope for bezier intersection using fat-line clipping
// Flatten the list of location arrays to one array and return it. // Flatten the list of location arrays to one array and return it.
locations = []; locations = [];
for (var i = 0, l = arrays.length; i < l; i++) { for (var i = 0, l = arrays.length; i < l; i++) {
locations.push.apply(locations, arrays[i]); Base.push(locations, arrays[i]);
} }
return locations; return locations;
} }

View file

@ -405,14 +405,7 @@ var Path = PathItem.extend(/** @lends Path# */{
} }
if (append) { if (append) {
// Append them all at the end. // Append them all at the end.
// Use a loop as the best way to handle big arrays (see #1493). Base.push(segments, segs);
// 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 { } else {
// Insert somewhere else // Insert somewhere else
segments.splice.apply(segments, [index, 0].concat(segs)); segments.splice.apply(segments, [index, 0].concat(segs));

View file

@ -112,8 +112,8 @@ PathItem.inject(new function() {
function collect(paths) { function collect(paths) {
for (var i = 0, l = paths.length; i < l; i++) { for (var i = 0, l = paths.length; i < l; i++) {
var path = paths[i]; var path = paths[i];
segments.push.apply(segments, path._segments); Base.push(segments, path._segments);
curves.push.apply(curves, path.getCurves()); Base.push(curves, path.getCurves());
// See if all encountered segments in a path are overlaps, to // See if all encountered segments in a path are overlaps, to
// be able to separately handle fully overlapping paths. // be able to separately handle fully overlapping paths.
path._overlapsOnly = true; path._overlapsOnly = true;
@ -1236,7 +1236,7 @@ PathItem.inject(new function() {
clearCurveHandles(clearCurves); clearCurveHandles(clearCurves);
// Finally resolve self-intersections through tracePaths() // Finally resolve self-intersections through tracePaths()
paths = tracePaths(Base.each(paths, function(path) { paths = tracePaths(Base.each(paths, function(path) {
this.push.apply(this, path._segments); Base.push(this, path._segments);
}, [])); }, []));
} }
// Determine how to return the paths: First try to recycle the // Determine how to return the paths: First try to recycle the

View file

@ -249,7 +249,6 @@ test('After removing all segments of a selected path, it should still be selecte
}, true); }, true);
}); });
test('After simplifying a path using #simplify(), the path should stay fullySelected', function() { test('After simplifying a path using #simplify(), the path should stay fullySelected', function() {
var path = new Path(); var path = new Path();
for (var i = 0; i < 30; i++) { for (var i = 0; i < 30; i++) {
@ -612,11 +611,12 @@ 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'); equals(error != null, true, 'We expect this arcTo() command to throw an error');
}); });
test('#1493 Path#add with 1000000 segments', function() { test('Path#add() with a lot of segments (#1493)', function() {
var path = new Path(); var segments = [];
for (var i = 0; i < 1000000; i++) { for (var i = 0; i < 100000; i++) {
path.add(new Point(0, 0)); segments.push(new Point(0, 0));
} }
var path = new Path(segments);
path.clone(); path.clone();
expect(0); expect(0);
}); });