From f6e716bf614b102380bb16de4918766cf2534bba Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Wed, 30 Aug 2017 11:41:04 -0400 Subject: [PATCH] Perf improvements - Don't .bind() within expression evaluation - Naively cache parseColor --- src/style-spec/function/compile.js | 7 ++++--- src/style-spec/function/definitions/curve.js | 5 ++--- src/style-spec/function/definitions/index.js | 20 +++++++++---------- src/style-spec/function/definitions/let.js | 2 +- src/style-spec/function/definitions/match.js | 8 ++++---- src/style-spec/function/evaluation_context.js | 19 ++++++++++-------- 6 files changed, 32 insertions(+), 29 deletions(-) diff --git a/src/style-spec/function/compile.js b/src/style-spec/function/compile.js index e156a3bf99c..4aaad7afc5a 100644 --- a/src/style-spec/function/compile.js +++ b/src/style-spec/function/compile.js @@ -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}) `); diff --git a/src/style-spec/function/definitions/curve.js b/src/style-spec/function/definitions/curve.js index 450d2af9338..aa245dd828c 100644 --- a/src/style-spec/function/definitions/curve.js +++ b/src/style-spec/function/definitions/curve.js @@ -1,8 +1,7 @@ // @flow -const interpolationFactor = require('../interpolation_factor'); - const UnitBezier = require('@mapbox/unitbezier'); +const interpolationFactor = require('../interpolation_factor'); const { toString, NumberType @@ -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(',')}], diff --git a/src/style-spec/function/definitions/index.js b/src/style-spec/function/definitions/index.js index a81739aa5ce..449312832f9 100644 --- a/src/style-spec/function/definitions/index.js +++ b/src/style-spec/function/definitions/index.js @@ -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})` ] ] }, @@ -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), '-': { @@ -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)})` ]; } @@ -189,7 +189,7 @@ function defineBooleanOp(op) { } function fromContext(name: string) { - return (args) => `this.${name}(${args.join(', ')})`; + return (args) => `$this.${name}(${args.join(', ')})`; } diff --git a/src/style-spec/function/definitions/let.js b/src/style-spec/function/definitions/let.js index 96a6bf789c7..af19b319ba0 100644 --- a/src/style-spec/function/definitions/let.js +++ b/src/style-spec/function/definitions/let.js @@ -35,7 +35,7 @@ class Let implements Expression { return `(function (${names.map(Let.escape).join(', ')}) { return ${result}; - }.bind(this))(${values.join(', ')})`; + })(${values.join(', ')})`; } serialize() { diff --git a/src/style-spec/function/definitions/match.js b/src/style-spec/function/definitions/match.js index 4ee059c6b70..b365417e680 100644 --- a/src/style-spec/function/definitions/match.js +++ b/src/style-spec/function/definitions/match.js @@ -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) { @@ -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() { diff --git a/src/style-spec/function/evaluation_context.js b/src/style-spec/function/evaluation_context.js index 34cee5cbaf3..4bfa43e8782 100644 --- a/src/style-spec/function/evaluation_context.js +++ b/src/style-spec/function/evaluation_context.js @@ -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'); @@ -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) { @@ -145,8 +150,6 @@ module.exports = () => ({ _unitBezierCache: ({}: {[string]: UnitBezier}), evaluateCurve(input: number, stopInputs: Array, stopOutputs: Array, 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](); @@ -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]();