Skip to content

Commit

Permalink
Perf improvements
Browse files Browse the repository at this point in the history
 - Don't .bind() within expression evaluation
 - Naively cache parseColor
  • Loading branch information
Anand Thakker committed Aug 30, 2017
1 parent 5ba431e commit f6e716b
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 29 deletions.
7 changes: 4 additions & 3 deletions src/style-spec/function/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,10 @@ function compileExpression(

const compiled = parsed.compile();
if (typeof compiled === 'string') {
const fn = new Function('mapProperties', 'feature', `
mapProperties = mapProperties || {};
var props = feature && feature.properties || {};
const fn = new Function('$globalProperties', '$feature', `
$globalProperties = $globalProperties || {};
var $props = $feature && $feature.properties || {};
var $this = this;
return this.unwrap(${compiled})
`);

Expand Down
5 changes: 2 additions & 3 deletions src/style-spec/function/definitions/curve.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
// @flow

const interpolationFactor = require('../interpolation_factor');

const UnitBezier = require('@mapbox/unitbezier');
const interpolationFactor = require('../interpolation_factor');
const {
toString,
NumberType
Expand Down Expand Up @@ -148,7 +147,7 @@ class Curve implements Expression {

const interpolationType = this.type.kind.toLowerCase();

return `this.evaluateCurve(
return `$this.evaluateCurve(
${input},
[${labels.join(',')}],
[${outputs.join(',')}],
Expand Down
20 changes: 10 additions & 10 deletions src/style-spec/function/definitions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,18 +65,18 @@ CompoundExpression.register(expressions, {
'get': {
type: ValueType,
overloads: [
[[StringType], ([k]) => `this.get(props, ${k}, 'feature.properties')`],
[[StringType], ([k]) => `$this.get($props, ${k}, 'feature.properties')`],
[[StringType, ObjectType], ([k, obj]) =>
`this.get(${obj}, ${k})`
`$this.get(${obj}, ${k})`
]
]
},
'has': {
type: BooleanType,
overloads: [
[[StringType], ([k]) => `this.has(props, ${k}, 'feature.properties')`],
[[StringType], ([k]) => `$this.has($props, ${k}, 'feature.properties')`],
[[StringType, ObjectType], ([k, obj]) =>
`this.has(${obj}, ${k})`
`$this.has(${obj}, ${k})`
]
]
},
Expand All @@ -88,15 +88,15 @@ CompoundExpression.register(expressions, {
]
},
'properties': [ObjectType, [], () =>
'this.as(props, {kind: "Object"}, "feature.properties")'
'$this.as($props, {kind: "Object"}, "feature.properties")'
],
'geometry-type': [ StringType, [], () =>
'this.geometryType(feature)'
'$this.geometryType($feature)'
],
'id': [ ValueType, [], () =>
'this.get(feature, "id", "feature")'
'$this.get($feature, "id", "feature")'
],
'zoom': [ NumberType, [], () => 'mapProperties.zoom' ],
'zoom': [ NumberType, [], () => '$globalProperties.zoom' ],
'+': defineBinaryMathOp('+', true),
'*': defineBinaryMathOp('*', true),
'-': {
Expand Down Expand Up @@ -149,7 +149,7 @@ CompoundExpression.register(expressions, {

function defineAssertion(type: Type) {
return [ type, [ValueType], (args) =>
`this.as(${args[0]}, ${JSON.stringify(type)})`
`$this.as(${args[0]}, ${JSON.stringify(type)})`
];
}

Expand Down Expand Up @@ -189,7 +189,7 @@ function defineBooleanOp(op) {
}

function fromContext(name: string) {
return (args) => `this.${name}(${args.join(', ')})`;
return (args) => `$this.${name}(${args.join(', ')})`;
}


Expand Down
2 changes: 1 addition & 1 deletion src/style-spec/function/definitions/let.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class Let implements Expression {

return `(function (${names.map(Let.escape).join(', ')}) {
return ${result};
}.bind(this))(${values.join(', ')})`;
})(${values.join(', ')})`;
}

serialize() {
Expand Down
8 changes: 4 additions & 4 deletions src/style-spec/function/definitions/match.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ class Match implements Expression {

compile() {
const input = this.input.compile();
const outputs = [`function () { return ${this.otherwise.compile()} }.bind(this)`];
const outputs = [`function () { return ${this.otherwise.compile()} }`];
const lookup = {};

for (const label in this.cases) {
Expand All @@ -106,15 +106,15 @@ class Match implements Expression {
lookup[`${String(label)}`] = this.cases[label] + 1;
}
for (const output of this.outputs) {
outputs.push(`function () { return ${output.compile()} }.bind(this)`);
outputs.push(`function () { return ${output.compile()} }`);
}

return `(function () {
var o = [${outputs.join(', ')}];
var l = ${JSON.stringify(lookup)};
var i = ${input};
return o[l[this.as(i, ${JSON.stringify(this.inputType)})] || 0]();
}.bind(this))()`;
return o[l[$this.as(i, ${JSON.stringify(this.inputType)})] || 0]();
})()`;
}

serialize() {
Expand Down
19 changes: 11 additions & 8 deletions src/style-spec/function/evaluation_context.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
const assert = require('assert');
const parseColor = require('../util/parse_color');
const interpolate = require('../util/interpolate');
const {toString, NumberType, ObjectType} = require('./types');
const {toString, ObjectType} = require('./types');
const {Color, typeOf, isValue} = require('./values');
const checkSubtype = require('./check_subtype');
const Curve = require('./definitions/curve');
Expand Down Expand Up @@ -102,11 +102,16 @@ module.exports = () => ({
}
},

_parseColorCache: ({}: {[string]: Color}),
parseColor: function (input: string) {
const c = parseColor(input);
if (!c)
throw new RuntimeError(`Could not parse color from value '${input}'`);
return new Color(...c);
let cached = this._parseColorCache[input];
if (!cached) {
const c = parseColor(input);
if (!c)
throw new RuntimeError(`Could not parse color from value '${input}'`);
cached = this._parseColorCache[input] = new Color(...c);
}
return cached;
},

rgba: function (r: number, g: number, b: number, a?: number) {
Expand Down Expand Up @@ -145,8 +150,6 @@ module.exports = () => ({

_unitBezierCache: ({}: {[string]: UnitBezier}),
evaluateCurve(input: number, stopInputs: Array<number>, stopOutputs: Array<Function>, interpolation: InterpolationType, resultType: string) {
input = this.as(input, NumberType, 'curve input');

const stopCount = stopInputs.length;
if (stopInputs.length === 1) return stopOutputs[0]();
if (input <= stopInputs[0]) return stopOutputs[0]();
Expand All @@ -160,7 +163,7 @@ module.exports = () => ({

const lower = stopInputs[index];
const upper = stopInputs[index + 1];
const t = Curve.interpolationFactor(interpolation, input, lower, upper);
const t = Curve.interpolationFactor(interpolation, input, lower, upper, this._unitBezierCache);

const outputLower = stopOutputs[index]();
const outputUpper = stopOutputs[index + 1]();
Expand Down

0 comments on commit f6e716b

Please sign in to comment.