Optimized some slow parts of the engine.

Last low hanging fruit: parsing is slow, cache parsed calculations
2021-09-29 15:54:14 +02:00
parent cb10b53a10
commit cb1fd38df3
21 changed files with 151 additions and 96 deletions
--- a/app/imports/api/creature/creatureProperties/CreatureProperties.js
+++ b/app/imports/api/creature/creatureProperties/CreatureProperties.js
@@ -55,6 +55,7 @@ const DenormalisedOnlyCreaturePropertySchema = new SimpleSchema({
    type: Boolean,
    optional: true,
    index: 1,
+    removeBeforeCompute: true,
  },
  // Denormalised flag if this property was made inactive by an inactive
  // ancestor. True if this property has an inactive ancestor even if this
@@ -63,6 +64,7 @@ const DenormalisedOnlyCreaturePropertySchema = new SimpleSchema({
    type: Boolean,
    optional: true,
    index: 1,
+    removeBeforeCompute: true,
  },
  // Denormalised flag if this property was made inactive because of its own
  // state
@@ -70,6 +72,7 @@ const DenormalisedOnlyCreaturePropertySchema = new SimpleSchema({
    type: Boolean,
    optional: true,
    index: 1,
+    removeBeforeCompute: true,
  },
  // Denormalised flag if this property was made inactive because of a toggle
  // calculation. Either an ancestor toggle calculation or its own.
@@ -77,6 +80,7 @@ const DenormalisedOnlyCreaturePropertySchema = new SimpleSchema({
    type: Boolean,
    optional: true,
    index: 1,
+    removeBeforeCompute: true,
  },
 });

--- a/app/imports/api/engine/computation/CreatureComputation.js
+++ b/app/imports/api/engine/computation/CreatureComputation.js
@@ -1,4 +1,4 @@
-import { cloneDeep } from 'lodash';
+import { EJSON } from 'meteor/ejson';
 import createGraph from 'ngraph.graph';

 export default class CreatureComputation {
@@ -6,8 +6,6 @@ export default class CreatureComputation {
    // Set up fields
    this.originalPropsById =  {};
    this.propsById =  {};
-    this.propsByType =  {};
-    this.propsByVariableName =  {};
    this.scope = {};
    this.props =  properties;
    this.dependencyGraph = createGraph();
@@ -15,21 +13,11 @@ export default class CreatureComputation {
    // Store properties for easy access later
    properties.forEach(prop => {
      // Store a copy of the unmodified prop
-      this.originalPropsById[prop._id] = cloneDeep(prop);
-
+      // EJSON clone is ~4x faster than lodash cloneDeep for EJSONable objects
+      this.originalPropsById[prop._id] = EJSON.clone(prop);
      // Store by id
      this.propsById[prop._id] = prop;

-      // Store by type
-      this.propsByType[prop.type] ?
-        this.propsByType[prop.type].push(prop) :
-        this.propsByType[prop.type] = [prop];
-
-      // Store by variableName
-      this.propsByVariableName[prop.variableName] ?
-        this.propsByVariableName[prop.variableName].push(prop) :
-        this.propsByVariableName[prop.variableName]= [prop];
-
      // Store the prop in the dependency graph
      this.dependencyGraph.addNode(prop._id, prop);
    });
--- a/app/imports/api/engine/computation/buildComputation/parseCalculationFields.js
+++ b/app/imports/api/engine/computation/buildComputation/parseCalculationFields.js
@@ -11,66 +11,53 @@ export default function parseCalculationFields(prop, schemas){

 function discoverInlineCalculationFields(prop, schemas){
  // For each key in the schema
-  schemas[prop.type]._schemaKeys.forEach( key => {
+  schemas[prop.type].inlineCalculationFields().forEach( calcKey => {
    // That ends in .inlineCalculations
-    if (key.slice(-19) === '.inlineCalculations'){
-      const inlineCalcKey = key.slice(0, -19);
-      applyFnToKey(prop, inlineCalcKey, (prop, key) => {
-        const inlineCalcObj = get(prop, key);
-        if (!inlineCalcObj) return;
-        // Store a reference to all the inline calculations
-        prop._computationDetails.inlineCalculations.push(inlineCalcObj);
-        // Extract the calculations and store them on the property
-        let string = inlineCalcObj.text;
-        if (!string) return;
-        inlineCalcObj.inlineCalculations = [];
-        let matches = string.matchAll(INLINE_CALCULATION_REGEX);
-        for (let match of matches){
-          let calculation = match[1];
-          inlineCalcObj.inlineCalculations.push({
-            calculation,
-          });
-        }
-      });
-    }
+    applyFnToKey(prop, calcKey, (prop, key) => {
+      const inlineCalcObj = get(prop, key);
+      if (!inlineCalcObj) return;
+      // Store a reference to all the inline calculations
+      prop._computationDetails.inlineCalculations.push(inlineCalcObj);
+      // Extract the calculations and store them on the property
+      let string = inlineCalcObj.text;
+      if (!string) return;
+      inlineCalcObj.inlineCalculations = [];
+      let matches = string.matchAll(INLINE_CALCULATION_REGEX);
+      for (let match of matches){
+        let calculation = match[1];
+        inlineCalcObj.inlineCalculations.push({
+          calculation,
+        });
+      }
+    });
  });
 }

 function parseAllCalculationFields(prop, schemas){
-  // For each key in the schema
-  schemas[prop.type]._schemaKeys.forEach( key => {
-    //  that ends in '.calculation'
-    if (key.slice(-12) === '.calculation'){
-      const calcKey = key.slice(0, -12);
-      // Determine the level the calculation should compute down to
-      let parseLevel = schemas[prop.type].getDefinition(calcKey).parseLevel || 'reduce';
+  // For each computed key in the schema
+  schemas[prop.type].computedFields().forEach( calcKey => {
+    // Determine the level the calculation should compute down to
+    let parseLevel = schemas[prop.type].getDefinition(calcKey).parseLevel || 'reduce';

-      // For all fields matching they keys
-      // supports `keys.$.with.$.arrays`
-      applyFnToKey(prop, calcKey, (prop, key) => {
-        const calcObj = get(prop, key);
-        if (!calcObj) return;
-        // If the calculation isn't set, delete the whole object
-        if (!calcObj.calculation){
-          unset(prop, key);
-          return;
-        }
-        // Store a reference to all the calculations
-        prop._computationDetails.calculations.push(calcObj);
-        // Store the level to compute down to later
-        calcObj._parseLevel = parseLevel;
-        // Parse the calculation
-        parseCalculation(calcObj);
-      });
-      // Or that ends in .inlineCalculations
-    }
+    // For all fields matching they keys
+    // supports `keys.$.with.$.arrays`
+    applyFnToKey(prop, calcKey, (prop, key) => {
+      const calcObj = get(prop, key);
+      if (!calcObj) return;
+      // Store a reference to all the calculations
+      prop._computationDetails.calculations.push(calcObj);
+      // Store the level to compute down to later
+      calcObj._parseLevel = parseLevel;
+      // Parse the calculation
+      parseCalculation(calcObj);
+    });
  });
 }

 function parseCalculation(calcObj){
-  let calculation = calcObj.calculation || '';
+  if (!calcObj.calculation) return;
  try {
-    calcObj._parsedCalculation = parse(calculation);
+    calcObj._parsedCalculation = parse(calcObj.calculation);
  } catch (e) {
    let error = {
      type: 'evaluation',
--- a/app/imports/api/engine/computation/buildComputation/removeSchemaFields.js
+++ b/app/imports/api/engine/computation/buildComputation/removeSchemaFields.js
@@ -3,17 +3,8 @@ import { unset } from 'lodash';

 export default function removeSchemaFields(schemas, prop){
  schemas.forEach(schema => {
-    schema._schemaKeys.forEach(key => {
-      // Skip object and array keys, except the errors array
-      if (
-        schema.getQuickTypeForKey(key) === 'object' ||
-        (
-          schema.getQuickTypeForKey(key) === 'objectArray' &&
-          key.slice(-6)!== 'errors'
-        )
-      ) return;
-      // Unset other computed only keys
-      applyFnToKey(prop, key, unset)
-    });
+    schema.removeBeforeComputeFields().forEach(
+      key => applyFnToKey(prop, key, unset)
+    );
  });
 }
--- a/app/imports/api/engine/computation/buildCreatureComputation.js
+++ b/app/imports/api/engine/computation/buildCreatureComputation.js
@@ -29,14 +29,10 @@ import removeSchemaFields from './buildComputation/removeSchemaFields.js';
 * computed toggles
 */

-/**
- * TODO
- * compute class levels
- */
-
 export default function buildCreatureComputation(creatureId){
  const properties = getProperties(creatureId);
-  return buildComputationFromProps(properties);
+  const computation = buildComputationFromProps(properties);
+  return computation;
 }

 function getProperties(creatureId){
--- a/app/imports/api/engine/computation/buildCreatureComputation.test.js
+++ b/app/imports/api/engine/computation/buildCreatureComputation.test.js
@@ -1,3 +1,4 @@
+import '/imports/api/simpleSchemaConfig.js';
 import { buildComputationFromProps } from './buildCreatureComputation.js';
 import { assert } from 'chai';
 import CreatureProperties from '/imports/api/creature/creatureProperties/CreatureProperties.js';
--- a/app/imports/api/engine/computation/computeComputation/computeByType/computeVariable/computeImplicitVariable.js
+++ b/app/imports/api/engine/computation/computeComputation/computeByType/computeVariable/computeImplicitVariable.js
@@ -7,9 +7,12 @@ import getAggregatorResult from './getAggregatorResult.js';
 export default function computeImplicitVariable(node){
   const prop = {};
   const result = getAggregatorResult(node);
-   prop.total = result;
-   prop.value = result;
-   prop.proficiency = node.data.proficiency;
+   if (result !== undefined){
+     prop.value = result;
+   }
+   if (node.data.proficiency !== undefined){
+     prop.proficiency = node.data.proficiency;
+   }

   // denormalise class level aggregator
   let classLevelAgg = node.data.classLevelAggregator;
--- a/app/imports/api/engine/computation/computeComputation/computeCalculations.js
+++ b/app/imports/api/engine/computation/computeComputation/computeCalculations.js
@@ -35,6 +35,7 @@ function evaluateCalculation(calculation, scope){
  // remove the working fields
  delete calculation._parseLevel;
  delete calculation._parsedCalculation;
+  delete calculation._localScope;
 }

 function embedInlineCalculations(inlineCalcObj){
--- a/app/imports/api/engine/computation/writeComputation/writeAlteredProperties.js
+++ b/app/imports/api/engine/computation/writeComputation/writeAlteredProperties.js
@@ -1,5 +1,5 @@
 import { Meteor } from 'meteor/meteor'
-import { isEqual } from 'lodash';
+import { EJSON } from 'meteor/ejson';
 import CreatureProperties from '/imports/api/creature/creatureProperties/CreatureProperties.js';
 import propertySchemasIndex from '/imports/api/properties/computedOnlyPropertySchemasIndex.js';

@@ -35,7 +35,7 @@ function addChangedKeysToOp(op, keys, original, changed) {
  // Loop through all keys that can be changed by computation
  // and compile an operation that sets all those keys
  for (let key of keys){
-    if (!isEqual(original[key], changed[key])){
+    if (!EJSON.equals(original[key], changed[key])){
      if (!op) op = newOperation(original._id, changed.type);
      let value = changed[key];
      if (value === undefined){
@@ -91,7 +91,7 @@ function writePropertiesSequentially(bulkWriteOps){
      bypassCollection2: true,
    });
  });
-  if (bulkWriteOps.length) console.log(`Wrote ${bulkWriteOps.length} props`);
+  //if (bulkWriteOps.length) console.log(`Wrote ${bulkWriteOps.length} props`);
 }

 // This is more efficient on the database, but significantly less efficient
--- a/app/imports/api/engine/computeCreature.js
+++ b/app/imports/api/engine/computeCreature.js
@@ -3,11 +3,9 @@ import computeCreatureComputation from './computation/computeCreatureComputation
 import writeAlteredProperties from './computation/writeComputation/writeAlteredProperties.js';

 export default function computeCreature(creatureId){
-  console.time('Compute creature');
  const computation = buildCreatureComputation(creatureId);
  computeCreatureComputation(computation);
  writeAlteredProperties(computation);
-  console.timeEnd('Compute creature');
 }

 // For now just recompute the whole creature, TODO only recompute a single
--- a/app/imports/api/properties/Actions.js
+++ b/app/imports/api/properties/Actions.js
@@ -2,7 +2,6 @@ import SimpleSchema from 'simpl-schema';
 import createPropertySchema from '/imports/api/properties/subSchemas/createPropertySchema.js';
 import { storedIconsSchema } from '/imports/api/icons/Icons.js';
 import STORAGE_LIMITS from '/imports/constants/STORAGE_LIMITS.js';
-SimpleSchema.extendOptions(['parseLevel']);

 /*
 * Actions are things a character can do
@@ -128,6 +127,7 @@ const ComputedOnlyActionSchema = createPropertySchema({
  insufficientResources: {
    type: Boolean,
    optional: true,
+    removeBeforeCompute: true,
  },
  uses: {
    type: 'computedOnlyField',
@@ -137,6 +137,7 @@ const ComputedOnlyActionSchema = createPropertySchema({
  usesLeft: {
    type: Number,
    optional: true,
+    removeBeforeCompute: true,
  },
  // Resources
  resources: {
@@ -153,6 +154,7 @@ const ComputedOnlyActionSchema = createPropertySchema({
  'resources.itemsConsumed.$.available': {
    type: Number,
    optional: true,
+    removeBeforeCompute: true,
  },
  'resources.itemsConsumed.$.quantity': {
    type: 'computedOnlyField',
@@ -162,16 +164,19 @@ const ComputedOnlyActionSchema = createPropertySchema({
    type: String,
    max: STORAGE_LIMITS.name,
    optional: true,
+    removeBeforeCompute: true,
  },
  'resources.itemsConsumed.$.itemIcon': {
    type: storedIconsSchema,
    optional: true,
    max: STORAGE_LIMITS.icon,
+    removeBeforeCompute: true,
  },
  'resources.itemsConsumed.$.itemColor': {
    type: String,
    optional: true,
    max: STORAGE_LIMITS.color,
+    removeBeforeCompute: true,
  },
  'resources.attributesConsumed': {
    type: Array,
@@ -187,16 +192,19 @@ const ComputedOnlyActionSchema = createPropertySchema({
  'resources.attributesConsumed.$.available': {
    type: Number,
    optional: true,
+    removeBeforeCompute: true,
  },
  'resources.attributesConsumed.$.statId': {
    type: String,
    regEx: SimpleSchema.RegEx.Id,
    optional: true,
+    removeBeforeCompute: true,
  },
  'resources.attributesConsumed.$.statName': {
    type: String,
    optional: true,
    max: STORAGE_LIMITS.name,
+    removeBeforeCompute: true,
  },
 });

--- a/app/imports/api/properties/Attributes.js
+++ b/app/imports/api/properties/Attributes.js
@@ -94,38 +94,45 @@ let ComputedOnlyAttributeSchema = createPropertySchema({
    type: SimpleSchema.oneOf(Number, String, Boolean),
 		defaultValue: 0,
    optional: true,
+    removeBeforeCompute: true,
  },
  // The computed value of the attribute minus the damage
  value: {
    type: SimpleSchema.oneOf(Number, String, Boolean),
 		defaultValue: 0,
    optional: true,
+    removeBeforeCompute: true,
  },
 	// The computed modifier, provided the attribute type is `ability`
 	modifier: {
 		type: SimpleSchema.Integer,
 		optional: true,
+    removeBeforeCompute: true,
 	},
  // Attributes with proficiency grant it to all skills based on the attribute
  proficiency: {
 		type: Number,
    allowedValues: [0.49, 0.5, 1, 2],
 		optional: true,
+    removeBeforeCompute: true,
 	},
  // The computed creature constitution modifier for hit dice
  constitutionMod: {
    type: Number,
 		optional: true,
+    removeBeforeCompute: true,
  },
  // Should this attribute hide
  hide: {
    type: Boolean,
    optional: true,
+    removeBeforeCompute: true,
  },
  // Denormalised tag if stat is overridden by one with the same variable name
  overridden: {
    type: Boolean,
    optional: true,
+    removeBeforeCompute: true,
  },
 });

--- a/app/imports/api/properties/Buffs.js
+++ b/app/imports/api/properties/Buffs.js
@@ -47,6 +47,7 @@ let ComputedOnlyBuffSchema = createPropertySchema({
 		type: Number,
 		optional: true,
 		min: 0,
+    removeBeforeCompute: true,
 	},
 	appliedBy: {
 		type: Object,
--- a/app/imports/api/properties/Classes.js
+++ b/app/imports/api/properties/Classes.js
@@ -74,10 +74,12 @@ const ComputedOnlyClassSchema = createPropertySchema({
    level: {
      type: SimpleSchema.Integer,
      optional: true,
+      removeBeforeCompute: true,
    },
    missingLevels: {
      type: Array,
      optional: true,
+      removeBeforeCompute: true,
    },
    'missingLevels.$': {
      type: SimpleSchema.Integer,
--- a/app/imports/api/properties/Containers.js
+++ b/app/imports/api/properties/Containers.js
@@ -43,20 +43,24 @@ const ComputedOnlyContainerSchema = createPropertySchema({
  contentsWeight:{
    type: Number,
    optional: true,
+    removeBeforeCompute: true,
  },
  // Weight of all the carried contents (some sub-containers might not be carried)
  // zero if `contentsWeightless` is true
  carriedWeight:{
    type: Number,
    optional: true,
+    removeBeforeCompute: true,
  },
  contentsValue:{
    type: Number,
    optional: true,
+    removeBeforeCompute: true,
  },
  carriedValue:{
    type: Number,
    optional: true,
+    removeBeforeCompute: true,
  },
 });

--- a/app/imports/api/properties/Skills.js
+++ b/app/imports/api/properties/Skills.js
@@ -66,6 +66,7 @@ let ComputedOnlySkillSchema = createPropertySchema({
    type: Number,
 		defaultValue: 0,
    optional: true,
+    removeBeforeCompute: true,
  },
  // The result of baseValueCalculation
  baseValue: {
@@ -80,28 +81,33 @@ let ComputedOnlySkillSchema = createPropertySchema({
 	abilityMod: {
 		type: SimpleSchema.Integer,
 		optional: true,
+    removeBeforeCompute: true,
 	},
 	// Computed advantage/disadvantage
  advantage: {
    type: SimpleSchema.Integer,
    optional: true,
    allowedValues: [-1, 0, 1],
+    removeBeforeCompute: true,
  },
 	// Computed bonus to passive checks
  passiveBonus: {
    type: Number,
    optional: true,
+    removeBeforeCompute: true,
  },
 	// Computed proficiency multiplier
  proficiency: {
    type: Number,
    allowedValues: [0, 0.49, 0.5, 1, 2],
 		defaultValue: 0,
+    removeBeforeCompute: true,
  },
  // Compiled text of all conditional benefits
  conditionalBenefits: {
    type: Array,
    optional: true,
+    removeBeforeCompute: true,
  },
  'conditionalBenefits.$': {
    type: String,
@@ -110,16 +116,19 @@ let ComputedOnlySkillSchema = createPropertySchema({
  fail: {
    type: SimpleSchema.Integer,
    optional: true,
+    removeBeforeCompute: true,
  },
  // Should this attribute hide
  hide: {
    type: Boolean,
    optional: true,
+    removeBeforeCompute: true,
  },
  // Denormalised tag if stat is overridden by one with the same variable name
  overridden: {
    type: Boolean,
    optional: true,
+    removeBeforeCompute: true,
  },
 })

--- a/app/imports/api/properties/Slots.js
+++ b/app/imports/api/properties/Slots.js
@@ -105,10 +105,12 @@ const ComputedOnlySlotSchema = createPropertySchema({
  totalFilled: {
    type: SimpleSchema.Integer,
    defaultValue: 0,
+    removeBeforeCompute: true,
  },
  spaceLeft: {
    type: SimpleSchema.Integer,
    optional: true,
+    removeBeforeCompute: true,
  },
 });

--- a/app/imports/api/properties/subSchemas/computedField.js
+++ b/app/imports/api/properties/subSchemas/computedField.js
@@ -22,11 +22,13 @@ function computedOnlyField(field){
    [`${field}.value`]: {
      type: SimpleSchema.oneOf(String, Number),
      optional: true,
+      removeBeforeCompute: true,
    },
    [`${field}.errors`]: {
      type: Array,
      optional: true,
      maxCount: STORAGE_LIMITS.errorCount,
+      removeBeforeCompute: true,
    },
    [`${field}.errors.$`]:{
      type: ErrorSchema,
@@ -40,17 +42,31 @@ function computedOnlyField(field){
 function includeParentFields(field, schemaObj){
  const splitField = field.split('.');
  if (splitField.length === 1){
-    schemaObj[field] = {type: Object, optional: true};
+    schemaObj[field] = {
+      type: Object,
+      optional: true,
+      computedField: true,
+    };
    return;
  }
  let key = '';
  splitField.push('');
-  splitField.forEach(value => {
+  splitField.forEach((value, index) => {
    if (key){
      if (value === '$'){
-        schemaObj[key] = {type: Array, optional: true};
+        schemaObj[key] = {
+          type: Array,
+          optional: true
+        };
      } else {
-        schemaObj[key] = {type: Object, optional: true};
+        schemaObj[key] = {
+          type: Object,
+          optional: true,
+        };
+        // the last object is the computed field
+        if (index === splitField.length - 1){
+          schemaObj[key].computedField = true;
+        }
      }
      key += '.';
    }
--- a/app/imports/api/properties/subSchemas/createPropertySchema.js
+++ b/app/imports/api/properties/subSchemas/createPropertySchema.js
@@ -31,6 +31,11 @@ export default function createPropertySchema(definition){
          `computed field: '${key}' of '${def.type}' is expected to be optional`
        );
      }
+      if (def.removeBeforeCompute){
+        console.warn(
+          `computed field: '${key}' of '${def.type}' should not be removed before computation`
+        )
+      }
    }
  }

--- a/app/imports/api/properties/subSchemas/inlineCalculationField.js
+++ b/app/imports/api/properties/subSchemas/inlineCalculationField.js
@@ -10,6 +10,7 @@ function inlineCalculationFieldToCompute(field){
    [field]: {
      type: Object,
      optional: true,
+      inlineCalculationField: true,
    },
    [`${field}.text`]: {
      type: String,
@@ -25,20 +26,24 @@ function computedOnlyInlineCalculationField(field){
    [field]: {
      type: Object,
      optional: true,
+      inlineCalculationField: true,
    },
    [`${field}.value`]: {
      type: String,
      optional: true,
      max: STORAGE_LIMITS.inlineCalculationField,
+      removeBeforeCompute: true,
    },
    [`${field}.inlineCalculations`]: {
      type: Array,
      defaultValue: [],
      maxCount: STORAGE_LIMITS.inlineCalculationCount,
+      removeBeforeCompute: true,
    },
    [`${field}.inlineCalculations.$`]: {
      type: Object,
      parseLevel: 'compile',
+      computedField: true,
    },
    // The part between bracers {}
    [`${field}.inlineCalculations.$.calculation`]: {
--- a/app/imports/api/simpleSchemaConfig.js
+++ b/app/imports/api/simpleSchemaConfig.js
@@ -1,3 +1,30 @@
 import SimpleSchema from 'simpl-schema';

-SimpleSchema.extendOptions(['parseLevel']);
+SimpleSchema.extendOptions([
+  'parseLevel',
+  'removeBeforeCompute',
+  'inlineCalculationField',
+  'computedField',
+]);
+
+// Store a quick way of referencing keys that have specific tags === true
+function storeTaggedKeys(tag, fnName){
+  SimpleSchema.prototype[fnName] = function(){
+    if (!this['_' + fnName]){
+      this['_' + fnName] = [];
+      for (const key in this._schema){
+        if (this._schema[key][tag]){
+          this['_' + fnName].push(key);
+        }
+      }
+    }
+    return this['_' + fnName];
+  }
+}
+
+// Keys that should be deleted at the start of a computation
+storeTaggedKeys('removeBeforeCompute', 'removeBeforeComputeFields');
+// Keys that represent inline calculation objects
+storeTaggedKeys('inlineCalculationField', 'inlineCalculationFields');
+// Keys that represent computed field objects
+storeTaggedKeys('computedField', 'computedFields');