From 1a143930315498822733ec6d196ed11cfb92b512 Mon Sep 17 00:00:00 2001 From: Stefan Zermatten Date: Sun, 3 Oct 2021 20:59:04 +0200 Subject: [PATCH] Parsed calculations are now cached between calculations Parsing is one of the more expensive computations done to characters, so the parser results are now stored on the DB and only updated if they are dirty. A hash is used to determine if the calculation has changed since the last computation --- .../linkCalculationDependencies.js | 2 +- .../parseCalculationFields.js | 35 ++++++++++++++----- .../linkCalculationDependencies.testFn.js | 4 +-- .../computation/buildCreatureComputation.js | 1 - .../computeByType/computeAction.js | 1 + .../computeComputation/computeCalculations.js | 13 +++---- .../tests/computeAttribute.testFn.js | 4 --- .../api/engine/computation/utility/cyrb53.js | 14 ++++++++ .../properties/subSchemas/computedField.js | 16 +++++++++ .../subSchemas/inlineCalculationField.js | 26 +++++++++++++- app/imports/parser/parseTree/index.js | 6 ++-- app/imports/parser/resolve.js | 2 ++ 12 files changed, 95 insertions(+), 29 deletions(-) create mode 100644 app/imports/api/engine/computation/utility/cyrb53.js diff --git a/app/imports/api/engine/computation/buildComputation/linkCalculationDependencies.js b/app/imports/api/engine/computation/buildComputation/linkCalculationDependencies.js index 0e91f833..9df70b88 100644 --- a/app/imports/api/engine/computation/buildComputation/linkCalculationDependencies.js +++ b/app/imports/api/engine/computation/buildComputation/linkCalculationDependencies.js @@ -8,7 +8,7 @@ export default function linkCalculationDependencies(dependencyGraph, prop, {prop // ancestors: {} //this gets added if there are resolved ancestors }; // Traverse the parsed calculation looking for variable names - traverse(calcObj._parsedCalculation, node => { + traverse(calcObj.parseNode, node => { // Skip nodes that aren't symbols or accessors if (node.parseType !== 'symbol' && node.parseType !== 'accessor') return; // Link ancestor references as direct property dependencies diff --git a/app/imports/api/engine/computation/buildComputation/parseCalculationFields.js b/app/imports/api/engine/computation/buildComputation/parseCalculationFields.js index 1d824e85..500afeee 100644 --- a/app/imports/api/engine/computation/buildComputation/parseCalculationFields.js +++ b/app/imports/api/engine/computation/buildComputation/parseCalculationFields.js @@ -2,7 +2,8 @@ import INLINE_CALCULATION_REGEX from '/imports/constants/INLINE_CALCULTION_REGEX import { prettifyParseError, parse } from '/imports/parser/parser.js'; import applyFnToKey from '/imports/api/engine/computation/utility/applyFnToKey.js'; import { get } from 'lodash'; -import errorNode from '/imports/parser/parseTree/error.js' +import errorNode from '/imports/parser/parseTree/error.js'; +import cyrb53 from '/imports/api/engine/computation/utility/cyrb53.js'; export default function parseCalculationFields(prop, schemas){ discoverInlineCalculationFields(prop, schemas); @@ -20,7 +21,15 @@ function discoverInlineCalculationFields(prop, schemas){ prop._computationDetails.inlineCalculations.push(inlineCalcObj); // Extract the calculations and store them on the property let string = inlineCalcObj.text; - if (!string) return; + if (!string){ + delete inlineCalcObj.hash; + return; + } + const inlineCalcHash = cyrb53(inlineCalcObj.text); + if (inlineCalcHash === inlineCalcObj.hash){ + return; + } + inlineCalcObj.hash = inlineCalcHash; inlineCalcObj.inlineCalculations = []; let matches = string.matchAll(INLINE_CALCULATION_REGEX); for (let match of matches){ @@ -55,17 +64,27 @@ function parseAllCalculationFields(prop, schemas){ } function parseCalculation(calcObj){ - if (!calcObj.calculation) return; + // If there is no calculation clear the cached parse node and error + if (!calcObj.calculation){ + delete calcObj.hash; + delete calcObj.parseError; + return; + } + const calcHash = cyrb53(calcObj.calculation); + // If the cached parse calculation is equal to the calculation, skip + if (calcHash === calcObj.hash){ + return; + } + calcObj.hash = calcHash; try { - calcObj._parsedCalculation = parse(calcObj.calculation); + calcObj.parseNode = parse(calcObj.calculation); + delete calcObj.parseError; } catch (e) { let error = { type: 'evaluation', message: prettifyParseError(e), }; - calcObj.errors ? - calcObj.errors.push(error) : - calcObj.errors = [error]; - calcObj._parsedCalculation = errorNode.create({error}); + calcObj.parseError = error; + calcObj.parseNode = errorNode.create({error}); } } diff --git a/app/imports/api/engine/computation/buildComputation/tests/linkCalculationDependencies.testFn.js b/app/imports/api/engine/computation/buildComputation/tests/linkCalculationDependencies.testFn.js index 3996b5a3..f432d926 100644 --- a/app/imports/api/engine/computation/buildComputation/tests/linkCalculationDependencies.testFn.js +++ b/app/imports/api/engine/computation/buildComputation/tests/linkCalculationDependencies.testFn.js @@ -24,8 +24,8 @@ export default function(){ 'Variable references create dependencies even if the attributes don\'t exist' ); assert.equal( - prop('strengthId').baseValue.errors.length, 1, - 'Parse errors should be added to calculation errors' + prop('strengthId').baseValue.parseError.message, 'Unexpected end of input', + 'Parse errors should be stored on the calculation doc' ); } diff --git a/app/imports/api/engine/computation/buildCreatureComputation.js b/app/imports/api/engine/computation/buildCreatureComputation.js index 7cb54d30..e78b248c 100644 --- a/app/imports/api/engine/computation/buildCreatureComputation.js +++ b/app/imports/api/engine/computation/buildCreatureComputation.js @@ -90,6 +90,5 @@ export function buildComputationFromProps(properties){ linkTypeDependencies(dependencyGraph, prop, computation); linkCalculationDependencies(dependencyGraph, prop, computation); }); - return computation; } diff --git a/app/imports/api/engine/computation/computeComputation/computeByType/computeAction.js b/app/imports/api/engine/computation/computeComputation/computeByType/computeAction.js index dde680fc..9b318fa5 100644 --- a/app/imports/api/engine/computation/computeComputation/computeByType/computeAction.js +++ b/app/imports/api/engine/computation/computeComputation/computeByType/computeAction.js @@ -25,6 +25,7 @@ function computeResources(computation, node){ resources.attributesConsumed.forEach(attConsumed => { if (!attConsumed.variableName) return; const att = computation.scope[attConsumed.variableName]; + if (!att._id) return; attConsumed.available = att.value; attConsumed.statId = att._id; attConsumed.statName = att.name; diff --git a/app/imports/api/engine/computation/computeComputation/computeCalculations.js b/app/imports/api/engine/computation/computeComputation/computeCalculations.js index ccbaf79f..6b74b5e3 100644 --- a/app/imports/api/engine/computation/computeComputation/computeCalculations.js +++ b/app/imports/api/engine/computation/computeComputation/computeCalculations.js @@ -13,25 +13,20 @@ export default function computeCalculations(computation, node){ } function evaluateCalculation(calculation, scope){ - const parseNode = calculation._parsedCalculation; + const parseNode = calculation.parseNode; const fn = calculation._parseLevel; const calculationScope = {...calculation._localScope, ...scope}; const {result: resultNode, context} = resolve(fn, parseNode, calculationScope); - if (resultNode.parseType === 'constant'){ + calculation.errors = context.errors; + if (resultNode?.parseType === 'constant'){ calculation.value = resultNode.value; - } else if (resultNode.parseType === 'error'){ + } else if (resultNode?.parseType === 'error'){ calculation.value = null; } else { calculation.value = toString(resultNode); } - if (calculation.errors){ - calculation.errors = [...calculation.errors, ...context.errors] - } else { - calculation.errors = context.errors - } // remove the working fields delete calculation._parseLevel; - delete calculation._parsedCalculation; delete calculation._localScope; } diff --git a/app/imports/api/engine/computation/computeComputation/tests/computeAttribute.testFn.js b/app/imports/api/engine/computation/computeComputation/tests/computeAttribute.testFn.js index 9eb5858a..25f298bb 100644 --- a/app/imports/api/engine/computation/computeComputation/tests/computeAttribute.testFn.js +++ b/app/imports/api/engine/computation/computeComputation/tests/computeAttribute.testFn.js @@ -15,10 +15,6 @@ export default function(){ assert.equal(scope('strength').modifier, 1); assert.equal(prop('referencesDexId').value, 4); assert.equal(prop('hitDiceId').constitutionMod, 5); - assert.equal( - prop('parseErrorId').baseValue.errors.length, 1, - 'Parse errors should be added to calculation errors' - ); assert.equal( prop('parseErrorId').baseValue.value, null, 'Parse errors should null the value' diff --git a/app/imports/api/engine/computation/utility/cyrb53.js b/app/imports/api/engine/computation/utility/cyrb53.js new file mode 100644 index 00000000..d93cb1c8 --- /dev/null +++ b/app/imports/api/engine/computation/utility/cyrb53.js @@ -0,0 +1,14 @@ +// Simple hash function from +// https://stackoverflow.com/questions/7616461/generate-a-hash-from-string-in-javascript +// Don't use for security +export default function(str, seed = 0) { + let h1 = 0xdeadbeef ^ seed, h2 = 0x41c6ce57 ^ seed; + for (let i = 0, ch; i < str.length; i++) { + ch = str.charCodeAt(i); + h1 = Math.imul(h1 ^ ch, 2654435761); + h2 = Math.imul(h2 ^ ch, 1597334677); + } + h1 = Math.imul(h1 ^ (h1>>>16), 2246822507) ^ Math.imul(h2 ^ (h2>>>13), 3266489909); + h2 = Math.imul(h2 ^ (h2>>>16), 2246822507) ^ Math.imul(h1 ^ (h1>>>13), 3266489909); + return 4294967296 * (2097151 & h2) + (h1>>>0); +} diff --git a/app/imports/api/properties/subSchemas/computedField.js b/app/imports/api/properties/subSchemas/computedField.js index 28b538c6..e8cb1220 100644 --- a/app/imports/api/properties/subSchemas/computedField.js +++ b/app/imports/api/properties/subSchemas/computedField.js @@ -24,6 +24,22 @@ function computedOnlyField(field){ optional: true, removeBeforeCompute: true, }, + // A cache of the parse result of the calculation + [`${field}.parseNode`]: { + type: Object, + optional: true, + blackbox: true, + }, + // Set if there was an error parsing the calculation + [`${field}.parseError`]: { + type: ErrorSchema, + optional: true, + }, + // a hash of the calculation to see if the cached values need to be updated + [`${field}.hash`]: { + type: Number, + optional: true, + }, [`${field}.errors`]: { type: Array, optional: true, diff --git a/app/imports/api/properties/subSchemas/inlineCalculationField.js b/app/imports/api/properties/subSchemas/inlineCalculationField.js index 71b49d50..7be26ed7 100644 --- a/app/imports/api/properties/subSchemas/inlineCalculationField.js +++ b/app/imports/api/properties/subSchemas/inlineCalculationField.js @@ -28,6 +28,12 @@ function computedOnlyInlineCalculationField(field){ optional: true, inlineCalculationField: true, }, + // a hash of the text to see if the current cached values need to be updated + [`${field}.hash`]: { + type: String, + optional: true, + max: STORAGE_LIMITS.inlineCalculationField, + }, [`${field}.value`]: { type: String, optional: true, @@ -38,7 +44,6 @@ function computedOnlyInlineCalculationField(field){ type: Array, defaultValue: [], maxCount: STORAGE_LIMITS.inlineCalculationCount, - removeBeforeCompute: true, }, [`${field}.inlineCalculations.$`]: { type: Object, @@ -50,15 +55,34 @@ function computedOnlyInlineCalculationField(field){ type: String, max: STORAGE_LIMITS.calculation, }, + // The result of the calc [`${field}.inlineCalculations.$.value`]: { type: SimpleSchema.oneOf(String, Number), optional: true, max: STORAGE_LIMITS.calculation, + removeBeforeCompute: true, + }, + // A cache of the parse result of the calculation + [`${field}.inlineCalculations.$.parseNode`]: { + type: Object, + optional: true, + blackbox: true, + }, + // Set if there was an error parsing the calculation + [`${field}.inlineCalculations.$.parseError`]: { + type: ErrorSchema, + optional: true, + }, + // a hash of the calculation to see if the cached values need to be updated + [`${field}.inlineCalculations.$.hash`]: { + type: Number, + optional: true, }, [`${field}.inlineCalculations.$.errors`]: { type: Array, optional: true, maxCount: STORAGE_LIMITS.errorCount, + removeBeforeCompute: true, }, [`${field}.inlineCalculations.$.errors.$`]: { type: ErrorSchema, diff --git a/app/imports/parser/parseTree/index.js b/app/imports/parser/parseTree/index.js index 17b00a21..8fbea29d 100644 --- a/app/imports/parser/parseTree/index.js +++ b/app/imports/parser/parseTree/index.js @@ -1,7 +1,7 @@ import resolve, { traverse, toString } from '../resolve'; import error from './error'; -const index = { +const indexNode = { create({array, index}) { return { parseType: 'index', @@ -53,7 +53,7 @@ const index = { } } return { - result: index.create({ + result: indexNode.create({ index, array, }), @@ -70,4 +70,4 @@ const index = { }, } -export default index; +export default indexNode; diff --git a/app/imports/parser/resolve.js b/app/imports/parser/resolve.js index add29f25..2663dbbd 100644 --- a/app/imports/parser/resolve.js +++ b/app/imports/parser/resolve.js @@ -3,6 +3,7 @@ import nodeTypeIndex from './parseTree/_index.js'; // Takes a parse ndoe and computes it to a set detail level // returns {result, context} export default function resolve(fn, node, scope, context = new Context()){ + if (!node) return {result: undefined, context}; let type = nodeTypeIndex[node.parseType]; if (!type){ throw new Meteor.Error(`Parse node type: ${node.parseType} not implemented`); @@ -21,6 +22,7 @@ export default function resolve(fn, node, scope, context = new Context()){ } export function toString(node){ + if (!node) return ''; let type = nodeTypeIndex[node.parseType]; if (!type.toString){ throw new Meteor.Error('toString not implemented on ' + node.parseType);