Skip to content

Commit

Permalink
Freeze cached Sim objects
Browse files Browse the repository at this point in the history
We're hunting ~~wabbits~~ validator bugs.

Honestly, this has been a long time coming, but Object.freeze perf
used to not be good enough for us to use it here. Here's hoping!
  • Loading branch information
Zarel committed Dec 7, 2023
1 parent ea967d4 commit 9cd64cb
Show file tree
Hide file tree
Showing 9 changed files with 32 additions and 15 deletions.
2 changes: 1 addition & 1 deletion lib/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ export abstract class Database<Pool extends mysql.Pool | pg.Pool = mysql.Pool |
const [query, values] = this._resolveSQL(sql);
return this._queryExec(query, values);
}
getTable<Row>(name: string, primaryKeyName: keyof Row & string | null = null) {
getTable<Row>(name: string, primaryKeyName: keyof Row & string | null = null): DatabaseTable<Row, this> {
return new DatabaseTable<Row, this>(this, name, primaryKeyName);
}
close() {
Expand Down
16 changes: 16 additions & 0 deletions lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,22 @@ export function deepClone(obj: any): any {
return clone;
}

export function deepFreeze<T>(obj: T): T {
if (obj === null || typeof obj !== 'object') return obj;
// support objects with reference loops
if (Object.isFrozen(obj)) return obj;

Object.freeze(obj);
if (Array.isArray(obj)) {
for (const elem of obj) deepFreeze(elem);
return obj;
}
for (const key of Object.keys(obj)) {

This comment has been minimized.

Copy link
@urkerab

urkerab Dec 7, 2023

Contributor

Is there a reason not to use Object.values directly?

This comment has been minimized.

Copy link
@Zarel

Zarel Dec 7, 2023

Author Member

No, that's better; feel free to push/PR.

deepFreeze((obj as any)[key]);
}
return obj;
}

export function levenshtein(s: string, t: string, l: number): number {
// Original levenshtein distance function by James Westgate, turned out to be the fastest
const d: number[][] = [];
Expand Down
2 changes: 1 addition & 1 deletion sim/dex-abilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export class DexAbilities {
});
}

if (ability.exists) this.abilityCache.set(id, ability);
if (ability.exists) this.abilityCache.set(id, this.dex.deepFreeze(ability));
return ability;
}

Expand Down
2 changes: 1 addition & 1 deletion sim/dex-conditions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,7 @@ export class DexConditions {
condition = new Condition({name: id, exists: false});
}

this.conditionCache.set(id, condition);
this.conditionCache.set(id, this.dex.deepFreeze(condition));
return condition;
}
}
8 changes: 4 additions & 4 deletions sim/dex-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ export class DexNatures {
nature = new Nature({name: id, exists: false});
}

if (nature.exists) this.natureCache.set(id, nature);
if (nature.exists) this.natureCache.set(id, this.dex.deepFreeze(nature));
return nature;
}

Expand All @@ -188,7 +188,7 @@ export class DexNatures {
for (const id in this.dex.data.Natures) {
natures.push(this.getByID(id as ID));
}
this.allCache = natures;
this.allCache = Object.freeze(natures);
return this.allCache;
}
}
Expand Down Expand Up @@ -278,7 +278,7 @@ export class DexTypes {
type = new TypeInfo({name: typeName, id, exists: false, effectType: 'EffectType'});
}

if (type.exists) this.typeCache.set(id, type);
if (type.exists) this.typeCache.set(id, this.dex.deepFreeze(type));
return type;
}

Expand All @@ -302,7 +302,7 @@ export class DexTypes {
for (const id in this.dex.data.TypeChart) {
types.push(this.getByID(id as ID));
}
this.allCache = types;
this.allCache = Object.freeze(types);
return this.allCache;
}
}
Expand Down
4 changes: 2 additions & 2 deletions sim/dex-items.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ export class DexItems {
item = new Item({name: id, exists: false});
}

if (item.exists) this.itemCache.set(id, item);
if (item.exists) this.itemCache.set(id, this.dex.deepFreeze(item));
return item;
}

Expand All @@ -212,7 +212,7 @@ export class DexItems {
for (const id in this.dex.data.Items) {
items.push(this.getByID(id as ID));
}
this.allCache = items;
this.allCache = Object.freeze(items);
return this.allCache;
}
}
4 changes: 2 additions & 2 deletions sim/dex-moves.ts
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,7 @@ export class DexMoves {
name: id, exists: false,
});
}
if (move.exists) this.moveCache.set(id, move);
if (move.exists) this.moveCache.set(id, this.dex.deepFreeze(move));
return move;
}

Expand All @@ -664,7 +664,7 @@ export class DexMoves {
for (const id in this.dex.data.Moves) {
moves.push(this.getByID(id as ID));
}
this.allCache = moves;
this.allCache = Object.freeze(moves);
return this.allCache;
}
}
8 changes: 4 additions & 4 deletions sim/dex-species.ts
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ export class DexSpecies {
}
}
}
this.speciesCache.set(id, species);
this.speciesCache.set(id, this.dex.deepFreeze(species));
return species;
}

Expand Down Expand Up @@ -516,7 +516,7 @@ export class DexSpecies {
exists: false, tier: 'Illegal', doublesTier: 'Illegal', natDexTier: 'Illegal', isNonstandard: 'Custom',
});
}
if (species.exists) this.speciesCache.set(id, species);
if (species.exists) this.speciesCache.set(id, this.dex.deepFreeze(species));
return species;
}

Expand All @@ -531,7 +531,7 @@ export class DexSpecies {
return new Learnset({exists: false});
}
learnsetData = new Learnset(this.dex.data.Learnsets[id]);
this.learnsetCache.set(id, learnsetData);
this.learnsetCache.set(id, this.dex.deepFreeze(learnsetData));
return learnsetData;
}

Expand All @@ -545,7 +545,7 @@ export class DexSpecies {
for (const id in this.dex.data.Pokedex) {
species.push(this.getByID(id as ID));
}
this.allCache = species;
this.allCache = Object.freeze(species);
return this.allCache;
}
}
1 change: 1 addition & 0 deletions sim/dex.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ export class ModdedDex {
textCache: TextTableData | null;

deepClone = Utils.deepClone;
deepFreeze = Utils.deepFreeze;

readonly formats: DexFormats;
readonly abilities: DexAbilities;
Expand Down

0 comments on commit 9cd64cb

Please sign in to comment.