Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Improve copy performance. #614

Merged
merged 6 commits into from

2 participants

@mojit0

Updated array checks to use Y.Lang.isArray to leverage native JS array tests.
Added new "blend" routine to avoid unnecessary overhead in copy/merge pairing.
Replaced copy/merge pairings with blend() calls.
Added unit tests for blend().
Removed duplicate mergeRecursive routines to avoid confusion/bugs.

mojit0 added some commits
@mojit0 mojit0 Improve copy performance.
Updated array checks to use Y.Lang.isArray to leverage native JS array tests.
Added new "blend" routine to avoid unnecessary overhead in copy/merge pairing.
Replaced copy/merge pairings with blend() calls.
Added unit tests for blend().
Removed duplicate mergeRecursive routines to avoid confusion/bugs.
256c840
@mojit0 mojit0 Merge remote-tracking branch 'upstream/develop-perf' into develop-perf 5f40602
@caridy

This block is not longer needed, was removed few days ago, not so sure why it is back.

I was repairing a problem where it would add without checks and didn't notice the removing during merge. Will remove.

@caridy
Owner

Just remove the block that is not needed, then +1

@mojit0 mojit0 merged commit 7a79647 into yahoo:develop-perf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 11, 2012
  1. @mojit0

    Improve copy performance.

    mojit0 authored
    Updated array checks to use Y.Lang.isArray to leverage native JS array tests.
    Added new "blend" routine to avoid unnecessary overhead in copy/merge pairing.
    Replaced copy/merge pairings with blend() calls.
    Added unit tests for blend().
    Removed duplicate mergeRecursive routines to avoid confusion/bugs.
  2. @mojit0
  3. @mojit0
  4. @mojit0
  5. @mojit0

    Removed extraneous code.

    mojit0 authored
  6. @mojit0

    Remove lint :(

    mojit0 authored
This page is out of date. Refresh to see the latest.
View
2  lib/app/addons/rs/config.server.js
@@ -103,7 +103,7 @@ YUI.add('addon-rs-config', function(Y, NAME) {
json,
ycb;
- ctx = store.mergeRecursive(store.getStaticContext(), ctx);
+ ctx = store.blendStaticContext(ctx);
if (!this._ycbCache[fullPath]) {
this._ycbCache[fullPath] = {};
View
91 lib/app/autoload/util.common.js
@@ -193,16 +193,19 @@ YUI.add('mojito-util', function(Y) {
copy: function(oldObj) {
var newObj,
- key;
+ key,
+ len,
+ copy = Y.mojito.util.copy;
if (!oldObj || typeof oldObj !== 'object') {
return oldObj;
}
- if ('[object Array]' === Object.prototype.toString.apply(oldObj)) {
+ if (Y.Lang.isArray(oldObj)) {
newObj = [];
- for (key = 0; key < oldObj.length; key += 1) {
- newObj[key] = Y.mojito.util.copy(oldObj[key]);
+ len = oldObj.length;
+ for (key = 0; key < len; key += 1) {
+ newObj[key] = copy(oldObj[key]);
}
return newObj;
}
@@ -210,7 +213,7 @@ YUI.add('mojito-util', function(Y) {
newObj = {};
for (key in oldObj) {
if (oldObj.hasOwnProperty(key)) {
- newObj[key] = Y.mojito.util.copy(oldObj[key]);
+ newObj[key] = copy(oldObj[key]);
}
}
return newObj;
@@ -225,6 +228,84 @@ YUI.add('mojito-util', function(Y) {
/**
+ * Blends a source base object and an overlay object to create a new
+ * object containing the recursively merged values of the two. This is
+ * similar to performing a copy and then a mergeRecursive operation but
+ * is intended to be more performant by avoiding duplicate work.
+ * @param {Object} base The base object. Properties in this object may
+ * be overwritten by those in overlay.
+ * @param {Object} extension The "overlay" object. Properties in this
+ * object are always found in the result.
+ * @return {Object} A new object containing the blended values.
+ */
+ blend: function(base, overlay) {
+
+ var tmp,
+ dest,
+ i,
+ slot,
+ slotType,
+ copy = Y.mojito.util.copy,
+ blend = Y.mojito.util.blend;
+
+ // Protect from bad input data.
+ if (base === null || base === undefined) {
+ return copy(overlay);
+ }
+ if (overlay === null || overlay === undefined) {
+ return copy(base);
+ }
+
+ if (Y.Lang.isArray(overlay)) {
+ if (!Y.Lang.isArray(base)) {
+ throw new Error('Type mismatch for object merge.');
+ }
+
+ tmp = base.slice(0);
+ tmp.push.apply(tmp, overlay);
+ dest = Y.Array.unique(tmp);
+ tmp.length = 0;
+ } else {
+ dest = {};
+ for (i in overlay) {
+ if (overlay.hasOwnProperty(i)) {
+ if (base.hasOwnProperty(i)) {
+ // "Conflicting" property. A recursive merge of the
+ // value is required if it's a complex object.
+ slot = overlay[i];
+ slotType = typeof slot;
+ if ((slotType === 'object') &&
+ (slot !== null)) {
+ // Complex, not null.
+ dest[i] = blend(base[i], overlay[i]);
+ } else {
+ // Simple value or null. Overlay value wins.
+ dest[i] = overlay[i];
+ }
+ } else {
+ // Unique to the overlay object, no merge needed.
+ dest[i] = copy(overlay[i]);
+ }
+ }
+ }
+
+ // Verify we didn't miss properties in the base. Anything
+ // missing from the destination object should be copied.
+ // Anything already there would have been merged in the previous
+ // loop.
+ for (i in base) {
+ if (base.hasOwnProperty(i)) {
+ if (!dest.hasOwnProperty(i)) {
+ dest[i] = copy(base[i]);
+ }
+ }
+ }
+ }
+ return dest;
+ },
+
+
+ /**
* Recursively merge properties of two objects
* @method mergeRecursive
* @param {object} dest The destination object.
View
29 lib/app/commands/test.js
@@ -277,35 +277,6 @@ function merge() {
}
-/**
- * Perform a deep merge.
- * @param {Object} dest The destination object.
- * @param {Object} src The source object.
- * @return {Object} The merged object.
- */
-function mergeRecursive(dest, src) {
- var p;
-
- for (p in src) {
- if (src.hasOwnProperty(p)) {
- try {
- // Property in destination object set; update its value.
- if (src[p].constructor === Object) {
- dest[p] = mergeRecursive(dest[p], src[p]);
- } else {
- dest[p] = src[p];
- }
- } catch (e) {
- // Property in destination object not set; create it and set its
- // value.
- dest[p] = src[p];
- }
- }
- }
- return dest;
-}
-
-
function preProcessor() {
var filepath,
View
3  lib/management/yui-module-configurator.js
@@ -118,8 +118,7 @@ module.exports = function(dir, excludes) {
return modules;
}
- // array test copied from Y.Lang.isArray()
- if ('[object Array]' !== Object.prototype.toString.call(dir)) {
+ if (Y.Lang.isArray(dir))
dir = [dir];
}
View
69 lib/store.server.js
@@ -288,6 +288,17 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
/**
+ * Returns the context provided blended with the static
+ * (non-runtime-sensitive) context.
+ * @method blendStaticContext
+ * @param {object} ctx The context to blend.
+ * @return {object} the context
+ */
+ blendStaticContext: function(ctx) {
+ return Y.mojito.util.blend(this._config.context, ctx);
+ },
+
+ /**
* Returns the static (non-runtime-sensitive) context
* @method getStaticContext
* @return {object} the context
@@ -334,12 +345,9 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
return Y.mojito.util.copy(this._appConfigCache[key]);
}
- // start with the base
- appConfig = Y.mojito.util.copy(this._fwConfig.appConfigBase);
-
- // apply the read values from the file
ycb = this.config.readConfigYCB(this._libs.path.join(this._config.root, 'application.json'), ctx);
- this.mergeRecursive(appConfig, ycb);
+
+ appConfig = Y.mojito.util.blend(this._fwConfig.appConfigBase, ycb);
this._appConfigCache[key] = appConfig;
@@ -523,10 +531,12 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
spec,
typeDetails,
config,
- perf;
+ perf,
+ newInst;
if (cacheValue) {
- cb(null, Y.mojito.util.mergeRecursive(Y.mojito.util.copy(cacheValue), instance));
+ newInst = Y.mojito.util.blend(cacheValue, instance);
+ cb(null, newInst);
return;
}
@@ -558,13 +568,12 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
perf.done();
}
if (spec.defaults && spec.defaults.config) {
- config = Y.mojito.util.copy(spec.defaults.config);
- this.mergeRecursive(config, spec.config);
+ config = Y.mojito.util.blend(spec.defaults.config, spec.config);
spec.config = config;
}
this._expandInstanceCache[env][cacheKey] = spec;
- cb(null, Y.mojito.util.mergeRecursive(Y.mojito.util.copy(spec), instance));
+ cb(null, Y.mojito.util.blend(spec, instance));
},
@@ -821,41 +830,6 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
return urls;
},
- /**
- * Recursively merge one object onto another.
- * [original implementation](http://stackoverflow.com/questions/171251/how-can-i-merge-properties-of-two-javascript-objects-dynamically/383245#383245)
- *
- * @method mergeRecursive
- * @param {object} dest object to merge into
- * @param {object} src object to merge onto "dest"
- * @param {boolean} typeMatch controls whether a non-object in the src is
- * allowed to clobber a non-object in the dest (if a different type)
- * @return {object} the modified "dest" object is also returned directly
- */
- mergeRecursive: function(dest, src, typeMatch) {
- var p;
- for (p in src) {
- if (src.hasOwnProperty(p)) {
- // Property in destination object set; update its value.
- if (src[p] && src[p].constructor === Object) {
- if (!dest[p]) {
- dest[p] = {};
- }
- dest[p] = this.mergeRecursive(dest[p], src[p]);
- } else {
- if (dest[p] && typeMatch) {
- if (typeof dest[p] === typeof src[p]) {
- dest[p] = src[p];
- }
- } else {
- dest[p] = src[p];
- }
- }
- }
- }
- return dest;
- },
-
//====================================================================
// CALLBACK METHODS
@@ -1468,7 +1442,10 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
}
delete spec.base;
- return this.mergeRecursive(this._expandSpec(env, ctx, base), spec);
+ return Y.mojito.util.mergeRecursive(
+ this._expandSpec(env, ctx, base),
+ spec
+ );
},
View
24 lib/tests/autoload/app/addons/rs/config-tests.server.js
@@ -36,30 +36,6 @@ YUI.add('mojito-addon-rs-config-tests', function(Y, NAME) {
return this._config.context || {};
},
- mergeRecursive: function(dest, src, typeMatch) {
- var p;
- for (p in src) {
- if (src.hasOwnProperty(p)) {
- // Property in destination object set; update its value.
- if (src[p] && src[p].constructor === Object) {
- if (!dest[p]) {
- dest[p] = {};
- }
- dest[p] = this.mergeRecursive(dest[p], src[p]);
- } else {
- if (dest[p] && typeMatch) {
- if (typeof dest[p] === typeof src[p]) {
- dest[p] = src[p];
- }
- } else {
- dest[p] = src[p];
- }
- }
- }
- }
- return dest;
- },
-
findResourceVersionByConvention: function(source, mojitType) {
// no-op
},
View
28 tests/unit/lib/app/addons/rs/test-config.server.js
@@ -3,8 +3,8 @@
* Copyrights licensed under the New BSD License.
* See the accompanying LICENSE file for terms.
*/
-YUI().use('addon-rs-config', 'base', 'oop', 'test', function(Y) {
-
+YUI().use('addon-rs-config', 'mojito-util', 'base', 'oop', 'test', function(Y) {
+
var suite = new YUITest.TestSuite('mojito-addon-rs-config-tests'),
libfs = require('fs'),
libpath = require('path'),
@@ -36,28 +36,8 @@ YUI().use('addon-rs-config', 'base', 'oop', 'test', function(Y) {
return this._config.context || {};
},
- mergeRecursive: function(dest, src, typeMatch) {
- var p;
- for (p in src) {
- if (src.hasOwnProperty(p)) {
- // Property in destination object set; update its value.
- if (src[p] && src[p].constructor === Object) {
- if (!dest[p]) {
- dest[p] = {};
- }
- dest[p] = this.mergeRecursive(dest[p], src[p]);
- } else {
- if (dest[p] && typeMatch) {
- if (typeof dest[p] === typeof src[p]) {
- dest[p] = src[p];
- }
- } else {
- dest[p] = src[p];
- }
- }
- }
- }
- return dest;
+ blendStaticContext: function(ctx) {
+ return Y.mojito.util.blend(this._config.context, ctx);
},
findResourceVersionByConvention: function(source, mojitType) {
View
109 tests/unit/lib/app/autoload/test-util.common.js
@@ -14,6 +14,115 @@
cases = {
name: 'functional',
+ 'blend should work on arrays': function() {
+ var base = [0, 1, 2, 3],
+ over = ['a', 'b'];
+
+ AA.itemsAreEqual([0, 1, 2, 3, 'a', 'b'],
+ Y.mojito.util.blend(base, over),
+ 'Array values should properly blend.');
+ },
+
+ 'blend should unique arrays': function() {
+ var base = [0, 1, 2, 3],
+ over = ['a', 'b', 1];
+
+ AA.itemsAreEqual([0, 1, 2, 3, 'a', 'b'],
+ Y.mojito.util.blend(base, over),
+ 'Array values should blend uniquely.');
+ },
+
+ 'blend should work on objects': function() {
+ var base = {
+ a: 1,
+ b: 2
+ },
+ over = {
+ c: 3,
+ d: 4
+ },
+ blended = {
+ a: 1,
+ b: 2,
+ c: 3,
+ d: 4
+ };
+
+ OA.areEqual(blended, Y.mojito.util.blend(base, over));
+ },
+
+ 'blend should replace object values': function() {
+ var base = {
+ a: 1,
+ b: 2
+ },
+ over = {
+ c: 3,
+ a: 4
+ },
+ blended = {
+ a: 4,
+ b: 2,
+ c: 3
+ };
+
+ OA.areEqual(blended, Y.mojito.util.blend(base, over));
+ },
+
+ 'blend should handle nested merges': function() {
+ var base = {
+ a: 1,
+ b: 2,
+ c: {
+ foo: 1
+ }
+ },
+ over = {
+ c: {
+ bar: 2
+ }
+ },
+ blended = {
+ a: 1,
+ b: 2,
+ c: {
+ foo: 1,
+ bar: 2
+ }
+ };
+
+ OA.areEqual(blended.c, Y.mojito.util.blend(base, over).c);
+ },
+
+ 'blend should handle nested merges with replacements': function() {
+ var base = {
+ a: 1,
+ b: 2,
+ c: {
+ foo: 1,
+ baz: 3
+ }
+ },
+ over = {
+ a: 4,
+ c: {
+ foo: 3,
+ bar: 2
+ }
+ },
+ blended = {
+ a: 4,
+ b: 2,
+ c: {
+ foo: 3,
+ bar: 2,
+ baz: 3
+ }
+ };
+
+ OA.areEqual(blended.c, Y.mojito.util.blend(base, over).c);
+ },
+
'cleanse should utf encode big 5 chars': function() {
A.areSame(
'test \\u003Cscript\\u003Ealert(\\u0022hi, i\\u0027m a squid \\u0026 a happy one!\\u0022)\\u003C/script\\u003E',
Something went wrong with that request. Please try again.