Skip to content
This repository has been archived by the owner on Oct 30, 2018. It is now read-only.

Commit

Permalink
Merge pull request #852 from drewfish/issue835-faster
Browse files Browse the repository at this point in the history 
address preformance regression introduced in pr 845
  • Loading branch information
@drewfish
drewfish committed Dec 11, 2012
2 parents 27a109f + 6081cc2 commit 9d6ceea
Show file tree
Hide file tree
Showing 5 changed files with 1,131 additions and 87 deletions.
55 changes: 18 additions & 37 deletions lib/app/autoload/store.server.js
View file
Expand Up @@ -563,7 +563,7 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
try {
perf = Y.mojito.perf.timeline('mojito', 'store:getMojitTypeDetails',
'expand mojit spec', spec.type);
typeDetails = this.getMojitTypeDetails(env, ctx, spec.type);
typeDetails = this.getMojitTypeDetails(env, ctx, spec.type, null, true);
} catch (err2) {
return cb(err2);
} finally {
Expand All @@ -574,8 +574,7 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
// * instance
// * spec
// * typeDetails
newInst = Y.mojito.util.blend(typeDetails, spec);
newInst = Y.mojito.util.mergeRecursive(newInst, instance);
newInst = Y.mojito.util.blend3(typeDetails, spec, instance);
cb(null, newInst);
},

Expand All @@ -586,11 +585,17 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
* As the last step of execution, this fires the `getMojitTypeDetails`
* event so that Resource Store addons can augment the returned structure.
*
* NOTE! This returns an object which is shared with similar calls to
* this method. If you intend to modify the object please make a deep
* copy first and use that instead.
*
* @method getMojitTypeDetails
* @param {string} env the runtime environment (either `client` or `server`)
* @param {object} ctx the context
* @param {string} mojitType mojit type
* @param {object} DEPRECATED: dest object in which to place the results
* @param {object} dest DEPRECATED: object in which to place the results
* @param {boolean} DANGERDANGERreturnRawCacheValue optional indicates
* that the cache value should be returned directly, instead of a copy. defaults to false.
* @return {object} details about the mojit type
*/
/**
Expand All @@ -604,7 +609,7 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
* @param {string} args.mojitType name of mojit
* @param {object} mojit the mojit type details
*/
getMojitTypeDetails: function(env, ctx, mojitType, dest) {
getMojitTypeDetails: function(env, ctx, mojitType, dest, DANGERDANGERreturnRawCacheValue) {
//Y.log('getMojitTypeDetails('+env+', '+JSON.stringify(ctx)+', '+mojitType+')', 'debug', NAME);
var posl = this.selector.getPOSLFromContext(ctx),
// We need to include the lang, since it's a part of the context
Expand Down Expand Up @@ -722,6 +727,9 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
Y.log('The "dest" parameter to store.getMojitTypeDetails() is deprecated.', 'warn', NAME);
Y.mojito.util.mergeRecursive(dest, cacheValue);
}
if (DANGERDANGERreturnRawCacheValue) {
return cacheValue;
}
return Y.mojito.util.copy(cacheValue);
},

Expand Down Expand Up @@ -1563,37 +1571,10 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
*/
// FUTURE: expose this to RS addons?
_expandSpec: function(env, ctx, spec) {
// [issue 835] The context is part of the cache key
// since the spec can vary in the application json.
var cacheKey = JSON.stringify([env, ctx, spec]),
cacheValue = this._expandSpecCache[cacheKey];

// We do the caching in this wrapper because the expandSpec
// algorithm is recursive and we don't want to keep doing
// the cache checks at each iteration.
// We -could- do caching within _expandSpecRecursive() for
// better cache re-use of expanded bases, but it's not
// (currently) expected that apps will have lots (1000s)
// of specs.
if (!cacheValue) {
cacheValue = this._expandSpecRecursive(env, ctx, spec);
this._expandSpecCache[cacheKey] = cacheValue;
}

return Y.mojito.util.copy(cacheValue);
},


/**
* Recursively build the spec by following base.
* @private
* @method _expandSpecRecursive
* @param {string} env the runtime environment (either `client` or `server`)
* @param {object} ctx runtime context
* @param {object} spec spec to expand
* @return {object} expanded sped
*/
_expandSpecRecursive: function(env, ctx, spec) {
// We could add caching in here, but it turns out that it's faster
// not to. This algorithm is pretty simple and a lot of the heavy
// lifting is being done inside the YCB library which has its own
// caching.
var appConfig,
base,
specParts,
Expand Down Expand Up @@ -1628,7 +1609,7 @@ YUI.add('mojito-resource-store', function(Y, NAME) {

spec.base = undefined;
return Y.mojito.util.mergeRecursive(
this._expandSpecRecursive(env, ctx, base),
this._expandSpec(env, ctx, base),
spec
);
},
Expand Down
180 changes: 147 additions & 33 deletions lib/app/autoload/util.common.js
View file
Expand Up @@ -9,7 +9,7 @@
/*global YUI*/


YUI.add('mojito-util', function(Y) {
YUI.add('mojito-util', function(Y, NAME) {

var META_AUTOCLOBBER = ['content-type'],
META_EXCLUDE = ['view'],
Expand Down Expand Up @@ -232,6 +232,8 @@ YUI.add('mojito-util', function(Y) {
* 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.
*
* @method blend
* @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
Expand All @@ -240,11 +242,9 @@ YUI.add('mojito-util', function(Y) {
*/
blend: function(base, overlay) {

var tmp,
dest,
var dest,
i,
slot,
slotType,
copy = Y.mojito.util.copy,
blend = Y.mojito.util.blend;

Expand All @@ -257,42 +257,40 @@ YUI.add('mojito-util', function(Y) {
}

if (Y.Lang.isArray(overlay)) {
if (!Y.Lang.isArray(base)) {
throw new Error('Type mismatch for object merge.');
if (Y.Lang.isArray(base)) {
return Y.Array.unique(base.concat(overlay));
}
Y.log('Type mismatch for array merge. Dropping value from base: ' + JSON.stringify(base), 'warn', NAME);
return overlay.slice(0);
}

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];
}
// Otherwise treat everything as an object.
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];
if (Y.Lang.isObject(slot)) {
// Complex, not null.
dest[i] = blend(base[i], slot);
} else {
// Unique to the overlay object, no merge needed.
dest[i] = copy(overlay[i]);
// Simple value or null. Overlay value wins.
dest[i] = slot;
}
} 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.
// 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.
if (Y.Lang.isObject(base) && !Y.Lang.isArray(base)) {
for (i in base) {
if (base.hasOwnProperty(i)) {
if (!dest.hasOwnProperty(i)) {
Expand All @@ -301,6 +299,122 @@ YUI.add('mojito-util', function(Y) {
}
}
}

return dest;
},


/**
* Blends three objects to create a new object containing the
* recursively merged values of the them all. This is similar to
* performing copy()s and then mergeRecursive()s but is intended
* to be more performant by avoiding duplicate work.
*
* The type of the "highest" parameter always dictates type of results.
*
* @method blend3
* @param {Object} lowest The lowest priority object.
* Properties in this object may be overwritten by those in medium and highest.
* @param {Object} medium The medium priority object.
* Properties in this object may be overwritten by those in highest.
* @param {Object} highest The highest priority object.
* Properties in this object are always found in the results.
* @return {Object} A new object containing the blended values.
*/
blend3: function(lowest, medium, highest) {
var dest,
key,
val,
useMedium,
useLowest,
copy = Y.mojito.util.copy,
blend2 = Y.mojito.util.blend,
blend3 = Y.mojito.util.blend3;

// Protect from bad input data.
if (highest === null || highest === undefined) {
return blend2(lowest, medium);
}

if (Y.Lang.isArray(highest)) {
if (Y.Lang.isArray(medium)) {
if (Y.Lang.isArray(lowest)) {
dest = lowest.concat(medium, highest);
} else {
dest = medium.concat(highest);
}
} else {
dest = highest.slice(0);
}
return Y.Array.unique(dest);
}

// Otherwise treat everything as an object.
dest = {};
useMedium = true;
useLowest = true;
if ((!Y.Lang.isObject(medium)) || Y.Lang.isArray(medium)) {
useMedium = false;
useLowest = false;
}
if (useLowest && ((!Y.Lang.isObject(lowest)) || Y.Lang.isArray(lowest))) {
useLowest = false;
}
for (key in highest) {
if (highest.hasOwnProperty(key)) {
val = highest[key];
if (Y.Lang.isObject(val)) {
if (useMedium && medium.hasOwnProperty(key)) {
if (useLowest && lowest.hasOwnProperty(key)) {
dest[key] = blend3(lowest[key], medium[key], val);
} else {
dest[key] = blend2(medium[key], val);
}
} else {
if (useLowest && lowest.hasOwnProperty(key)) {
dest[key] = blend2(lowest[key], val);
} else {
dest[key] = copy(val);
}
}
} else {
dest[key] = copy(val);
}
}
}

// Process keys that are in medium but not in highest.
if (useMedium) {
for (key in medium) {
if (medium.hasOwnProperty(key)) {
if (!dest.hasOwnProperty(key)) {
val = medium[key];
if (Y.Lang.isObject(val)) {
if (useLowest && lowest.hasOwnProperty(key)) {
dest[key] = blend2(lowest[key], val);
} else {
dest[key] = copy(val);
}
} else {
// scalar
dest[key] = val;
}
}
}
}
}

// Process keys that are in lowest but not highest nor in medium.
if (useLowest) {
for (key in lowest) {
if (lowest.hasOwnProperty(key)) {
if (!dest.hasOwnProperty(key)) {
dest[key] = copy(lowest[key]);
}
}
}
}

return dest;
},

Expand Down
32 changes: 31 additions & 1 deletion tests/base/mojito-test.js
View file
Expand Up @@ -212,6 +212,36 @@ YUI.add('mojito-test', function(Y, NAME) {


YUI.add('mojito-test-extra', function(Y, NAME) {
var A = Y.Assert;

Y.MOJITO_DIR = require ?
require('path').resolve(__dirname, '../../') + '/' : null;
});

// path doesn't need to be given, mainly used during recursion
Y.TEST_CMP = function (x, y, msg, path) {
path = path || 'obj';
var i;
if (Y.Lang.isArray(x)) {
A.isArray(x, path + ': ' + (msg || 'first arg should be an array'));
A.isArray(y, path + ': ' + (msg || 'second arg should be an array'));
A.areSame(x.length, y.length, path + ': ' + (msg || 'arrays are different lengths'));
for (i = 0; i < x.length; i += 1) {
Y.TEST_CMP(x[i], y[i], msg, path + '[' + i + ']');
}
return;
}
if (Y.Lang.isObject(x)) {
A.isObject(x, path + ': ' + (msg || 'first arg should be an object'));
A.isObject(y, path + ': ' + (msg || 'second arg should be an object'));
A.areSame(Object.keys(x).length, Object.keys(y).length, path + ': ' + (msg || 'object keys are different lengths'));
for (i in x) {
if (x.hasOwnProperty(i)) {
Y.TEST_CMP(x[i], y[i], msg, path + '{' + i + '}');
}
}
return;
}
A.areSame(x, y, path + ': ' + (msg || 'args should be the same'));
};

}, '0.1.0', {requires: ['test']});
15 changes: 0 additions & 15 deletions tests/unit/lib/app/autoload/test-store.server.js
View file
Expand Up @@ -261,21 +261,6 @@ YUI().use(
});
},

'expandSpec caching': function() {
var inSpec = {
base: 'a',
type: 'c'
};
var env = 'server';
var ctx = {};
var key = Y.JSON.stringify([env, ctx, inSpec]);
store._expandSpecCache[key] = { x: 'y' };
var outSpec = store._expandSpec(env, ctx, inSpec);
A.isObject(outSpec);
A.areEqual(1, Object.keys(outSpec).length);
A.areEqual('y', outSpec.x);
},

'getMojitTypeDetails caching': function() {
var key = Y.JSON.stringify(['server', ['*'], 'en', 'x']);
store._getMojitTypeDetailsCache[key] = { x: 'y' };
Expand Down

0 comments on commit 9d6ceea

Please sign in to comment.