Skip to content

Fix performance regression #292

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mattbaileyuk merged 2 commits into from
Jan 7, 2019
Merged

Fix performance regression #292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b82d167
improve performance of sequence wrapper
andrew-coleman Dec 21, 2018
af4fccd
fix timestamp format in unit tests
andrew-coleman Jan 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/functions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ const functions = (() => {
var isArrayOfStrings = utils.isArrayOfStrings;
var isArrayOfNumbers = utils.isArrayOfNumbers;
var createSequence = utils.createSequence;
// var isSequence = utils.isSequence;
// var toSequence = utils.toSequence;
var isFunction = utils.isFunction;
var isLambda = utils.isLambda;
var isIterable = utils.isIterable;
Expand Down
20 changes: 13 additions & 7 deletions src/jsonata.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ var jsonata = (function() {
var isArrayOfNumbers = utils.isArrayOfNumbers;
var createSequence = utils.createSequence;
var isSequence = utils.isSequence;
var toSequence = utils.toSequence;
var isFunction = utils.isFunction;
var isLambda = utils.isLambda;
var isIterable = utils.isIterable;
Expand Down Expand Up @@ -113,11 +112,11 @@ var jsonata = (function() {
break;
}

if(environment.lookup('__jsonata_async') &&
if(environment.async &&
(typeof result === 'undefined' || result === null || typeof result.then !== 'function')) {
result = Promise.resolve(result);
}
if(environment.lookup('__jsonata_async') && typeof result.then === 'function' && expr.nextFunction && typeof result[expr.nextFunction] === 'function') {
if(environment.async && typeof result.then === 'function' && expr.nextFunction && typeof result[expr.nextFunction] === 'function') {
// although this is a 'thenable', it is chaining a different function
// so don't yield since yielding will trigger the .then()
} else {
Expand All @@ -142,7 +141,12 @@ var jsonata = (function() {
if(expr.keepArray) {
result.keepSingleton = true;
}
result = result.value();
if(result.length === 0) {
result = undefined;
} else if(result.length === 1) {
result = result.keepSingleton ? result : result[0];
}

}

return result;
Expand Down Expand Up @@ -849,7 +853,8 @@ var jsonata = (function() {
for (var item = lhs, index = 0; item <= rhs; item++, index++) {
result[index] = item;
}
return toSequence(result);
result.sequence = true;
return result;
}

/**
Expand Down Expand Up @@ -1606,7 +1611,8 @@ var jsonata = (function() {
}
return value;
},
timestamp: enclosingEnvironment ? enclosingEnvironment.timestamp : null
timestamp: enclosingEnvironment ? enclosingEnvironment.timestamp : null,
async: enclosingEnvironment ? enclosingEnvironment.async : false
};
}

Expand Down Expand Up @@ -1854,7 +1860,7 @@ var jsonata = (function() {
var result, it;
// if a callback function is supplied, then drive the generator in a promise chain
if(typeof callback === 'function') {
exec_env.bind('__jsonata_async', true);
exec_env.async = true;
var catchHandler = function (err) {
populateMessage(err); // possible side-effects on `err`
callback(err, null);
Expand Down
51 changes: 3 additions & 48 deletions src/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ const utils = (() => {
* @returns {Array} - empty sequence
*/
function createSequence() {
var sequence = toSequence([]);
var sequence = [];
sequence.sequence = true;
if (arguments.length === 1) {
sequence.push(arguments[0]);
}
Expand All @@ -72,52 +73,7 @@ const utils = (() => {
* @returns {boolean} true if it's a sequence
*/
function isSequence(value) {
var sequence = value.sequence;
if(sequence === true) {
var desc = Object.getOwnPropertyDescriptor(value, 'sequence');
if(desc.enumerable === false) {
return true;
}
}
return false;
}

/**
* Converts an array to a result sequence (by adding special properties)
* @param {Array} arr - the array to convert
* @returns {*} - the sequence
*/
function toSequence(arr) {
Object.defineProperty(arr, 'sequence', {
enumerable: false,
configurable: false,
get: function () {
return true;
}
});
Object.defineProperty(arr, 'keepSingleton', {
enumerable: false,
configurable: false,
writable: true,
value: false
});
Object.defineProperty(arr, 'value', {
enumerable: false,
configurable: false,
get: function () {
return function() {
switch (this.length) {
case 0:
return undefined;
case 1:
return this.keepSingleton ? this : this[0];
default:
return this;
}
};
}
});
return arr;
return value.sequence === true && Array.isArray(value);
}

/**
Expand Down Expand Up @@ -175,7 +131,6 @@ const utils = (() => {
isArrayOfNumbers,
createSequence,
isSequence,
toSequence,
isFunction,
isLambda,
isIterable,
Expand Down
4 changes: 2 additions & 2 deletions test/implementation-tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ describe("Functions with side-effects", () => {

describe("$now() returns timestamp with defined format", function() {
it("should return result object", function() {
var expr = jsonata("$now('[h]:[M][P] [z]')");
var expr = jsonata("$now('[h]:[M01][P] [z]')");
Copy link
Member

@mattbaileyuk mattbaileyuk Jan 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is unrelated to the rest of the fixes, but agree with the change so not too worried that it's here.

var result = expr.evaluate(testdata2);
// follows this pattern - "10:23am GMT+00:00"
expect(result).to.match(/^\d?\d:\d\d[ap]m GMT\+00:00$/);
Expand All @@ -149,7 +149,7 @@ describe("Functions with side-effects", () => {

describe("$now() returns timestamp with defined format and timezone", function() {
it("should return result object", function() {
var expr = jsonata("$now('[h]:[M][P] [z]', '-0500')");
var expr = jsonata("$now('[h]:[M01][P] [z]', '-0500')");
var result = expr.evaluate(testdata2);
// follows this pattern - "10:23am GMT-05:00"
expect(result).to.match(/^\d?\d:\d\d[ap]m GMT-05:00$/);
Expand Down